APIcast icon indicating copy to clipboard operation
APIcast copied to clipboard

Mapping rules integration and verification with threescalers library

Open unleashed opened this issue 4 years ago • 0 comments

WARNING: This PR is NOT MEANT TO BE MERGED.

This is part of the work towards specifying mapping rules across client/proxy implementations and the 3scale administrative interface, see 3scale-rs/threescalers#79 for tracking information.

This branch adds the following:

  • [x] A minimal file README.threescalers.md explaining how to run these tests.
  • [x] A submodule with the threescalers library with an independent implementation of mapping rules logic and a C API.
  • [x] A development container image (in make development) that can build threescalers.
  • [x] Changes in Makefile to ensure the library is built when running make busted, along housekeeping targets.
  • [x] A gateway/src/threescalers folder importing the threescalers library via an FFI ABI.
  • [x] A spec/threescalers folder with specific unit tests for this library runnable via make threescalers-test.
  • [x] A set of additional specs for the mapping_rules_spec file taken from the threescalers tests[1].
  • [x] A small fix in the APIcast mapping rules implementation to avoid a crash under very specific conditions.
  • [x] An inline comparison between the APIcast and the threescalers implementation warning on STDOUT when they differ.
  • [x] Uses the threescalers implementation as final truth value, as it passes all previous specs and the added ones[2].
  • [x] One pending spec on matching escaped {} in the threescalers and APIcast specs that neither impl passes yet.
  • [x] Several pending specs regarding wildcards in query string parameter keys for APIcast[3].
  • [x] Several pending specs regarding inner and outer unbalanced and escaped wildcards[4].
Notes

[1] These need checking for any missing tests, but additionally, the way the Lua interface is designed limits how unit tests/specs can be written, so these lack some tests set as pending and might need further integration tests. In particular, there's no way to test wildcards in query string parameter keys (as opposed to values). [2] The current solution is not ideal because of the way the interface is designed using Lua tables and arrays. A threescalers only solution does not need to spend time preparing the data, since that is done behind the scenes by the library. [3] Note [1] above applies, but notice that some tests are defined in terms of what threescalers does at the moment, not in terms of what is correct to do, so they fail on APIcast because the implementation never considered some features. [4] See table below about wildcard edge cases.

Failures

The APIcast implementation, when changing the code to make it the source of truth, will fail several specs. While some of those are obviously missing cases in the implementation, others deal with what to do with query string parameters in special cases, which is undefined as there is no specification.

Current support comparison

Concept APIcast threescalers
Path prefixes :heavy_check_mark: :heavy_check_mark:
Dedup // in pattern :heavy_check_mark: :heavy_check_mark:
Dedup // in path :x: :heavy_check_mark:
Support for other wildcards :x: :x: (TBD)
Escaping braces :x: :heavy_check_mark:
Match on literal braces :x: :x: (bug/wip)
Escaping $ :x: :heavy_check_mark:
Non significant QS order :heavy_check_mark: :heavy_check_mark:
QS value wildcards :heavy_check_mark: :heavy_check_mark:
QS value partial wildcards :x: :heavy_check_mark:
QS value strictness :heavy_check_mark: (implied) :x: (TBD/wip via $)
QS value prefixes :x: :heavy_check_mark:
QS key wildcards :x: :heavy_check_mark:
QS key partial wildcards :x: :heavy_check_mark:
QS key strictness :heavy_check_mark: :heavy_check_mark:
QS key prefixes :x: :x: (future support:question:)

Wildcard edge case support

Concept APIcast threescalers
/_{wild{card}}_/ :x: :x:
/_{wild\\{card\\}}_/ :x: :x:
/_{wild{card}_/ :heavy_check_mark: :heavy_check_mark:
/_{wild}card}_/ :heavy_check_mark: :heavy_check_mark:
/_{wild\\{card}_/ :heavy_check_mark: :heavy_check_mark:
/_{wild\\}card}_/ :x: :x:
/_\\{wild{card}_/ :x: :x:
/_{wild}card\\}_/ :x: :x:
/_{wildcard_/ :question: :exclamation: (fails validation)
/_{wild}_{card}_/ :heavy_check_mark: :heavy_check_mark:

Mismatches

The code introduces a check in the mapping rules logic to compare Apicast and threescalers implementation, and outputs text to stdout if there is a mismatch. By default the code uses threescalers as it passes all the relevant non-pending specs, but this can be changed (and it is encouraged to do so to compare) at the end of the mapping_rules.lua#matching function.

A few mismatches happen when running make busted (whether setting one implementation or the other as source of truth, although the specific mismatching cases will be slightly different). The list below show the important different classes:

!!! Test         : { method: GET, uri: /abc/v1/id$/1//, qs_args:  }
!!! Apicast rule : { method: ANY, pattern: /abc/v{version}/id\$/{n}/$, params: nil, qs params:  }
!!! 3scalers rule: RestRule { method: Any, path: \A/abc/v[0-9a-zA-Z_\-.~%!$&'()*+,;=@:]+/id\$/[0-9a-zA-Z_\-.~%!$&'()*+,;=@:]+/$, qs: Some([\A]) }
  • APIcast -> :x:
  • 3scalers -> :heavy_check_mark:
  • Expected -> :heavy_check_mark:

This is successful for threescalers because it de-duplicates path separators from input paths before evaluating them, whereas APIcast might be confused by the escaped dollar sign or alternatively it matches both forward slashes. This currently causes a spec to fail if using APIcasts.

!!! Test         : { method: GET, uri: /abc, qs_args: s=1&t=$9&fmt=html&lang= }
!!! Apicast rule : { method: ANY, pattern: /abc, params: nil, qs params: s=1&t=$9&fmt={fmt}&lang={code} }
!!! 3scalers rule: RestRule { method: Any, path: \A/abc, qs: Some([\As=1, \At=\$9, \Afmt=[0-9a-zA-Z_\-.~%!$'()*+,;=@:]+, \Alang=[0-9a-zA-Z_\-.~%!$'()*+,;=@:]+]) }
  • APIcast -> :heavy_check_mark:
  • 3scalers -> :x:
  • Expected -> :x:

This fails on threescalers because the {...} notation requires that at least one character is present. This might be reinterpreted to be different for query string parameter values, but there is no specification around that making an exception. This currently causes a spec to fail if using APIcast's.

!!! Test         : { method: GET, uri: /abc, qs_args: lang=ca }
!!! APIcast rule : { method: ANY, pattern: /abc, params: nil, qs params: lang={code}t }
!!! 3scalers rule: RestRule { method: Any, path: \A/abc, qs: Some([\Alang=[0-9a-zA-Z_\-.~%!$'()*+,;=@:]+t]) }
  • APIcast -> :heavy_check_mark:
  • 3scalers -> :x:
  • Expected -> :x:

This fails on threescalers because it looks for a value in the lang parameter that has one or more characters before a t (which includes but is not limited to the case of ending with a t). Presumably the APIcast implementation just takes a wildcard for the whole value.

!!! Test         : { method: GET, uri: /abc, qs_args: lang=123 }
!!! APIcast rule : { method: ANY, pattern: /abc, params: nil, qs params: lang=1 }
!!! 3scalers rule: RestRule { method: Any, path: \A/abc, qs: Some([\Alang=1]) }
  • APIcast -> :x:
  • 3scalers -> :heavy_check_mark:
  • Expected -> :x: or :question:

This is the case of using parameter values as fully specified or as prefixes. This is unspecified, and although threescalers uses prefixes, turns out it understands $ as a literal, so the current implementation only matches prefixes. Conversely the APIcast implementation only matches strictly, so it cannot use prefixes. This is arguably more useful with strict matching unless an implementation supports strictness via $.

unleashed avatar Sep 06 '21 00:09 unleashed