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

Condition engine refactor and improvements — JSON Schema Validation #882

Merged
merged 20 commits into from
Mar 25, 2019

Conversation

XVincentX
Copy link
Member

@XVincentX XVincentX commented Mar 16, 2019

This PR contains a refactor of the condition engine solving couple of limitations that were starting be an issue.

As a result of the changes, we're now able to validate the condition's JSON Schema before starting the gateway (catching problems ahead of the time instead of catching them during the runtime).

Summary:

  1. Kill the matchEgCondition function. This helps making conditions and policy kind of the same thing; it should be easier to maintain.
  2. Changed the condition handler signature — with compatibility of the old one. This helps to prepare condition configuration and stuff before returning the actual function. It's nothing more than a layer of indirection.
  3. Tests updated and bla bla bla

I've also implemented the JSON Schema Validate condition which can be used around, helping moving the OpenAPI integration forward. Right now it works by using kind of a secret property called middlewares so that they can be installed before evaluating the current policy. Kind of hacky, but it does the job.

Connect #591
Closes #591

@XVincentX XVincentX force-pushed the feat/condition-refactor branch from 9156caf to 67b2633 Compare March 16, 2019 14:19
@XVincentX XVincentX force-pushed the feat/condition-refactor branch from 6c67292 to 5be5221 Compare March 16, 2019 14:46
@codecov
Copy link

codecov bot commented Mar 16, 2019

Codecov Report

Merging #882 into master will increase coverage by 0.01%.
The diff coverage is 90.32%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #882      +/-   ##
==========================================
+ Coverage   89.15%   89.16%   +0.01%     
==========================================
  Files         135      136       +1     
  Lines        3652     3674      +22     
==========================================
+ Hits         3256     3276      +20     
- Misses        396      398       +2
Impacted Files Coverage Δ
lib/gateway/index.js 96.49% <ø> (ø) ⬆️
lib/schemas/index.js 100% <ø> (ø) ⬆️
lib/policies/index.js 100% <100%> (ø) ⬆️
lib/gateway/pipelines.js 92.9% <100%> (+0.42%) ⬆️
lib/conditions/index.js 100% <100%> (+8.69%) ⬆️
lib/config/config.js 90.32% <100%> (+0.1%) ⬆️
lib/conditions/json-schema.js 63.63% <63.63%> (ø)
lib/conditions/predefined.js 89.28% <91.66%> (+5.07%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5003d60...69b39db. Read the comment docs.

@XVincentX XVincentX force-pushed the feat/condition-refactor branch from 04bc022 to d61ea23 Compare March 16, 2019 15:24
@XVincentX XVincentX force-pushed the feat/condition-refactor branch 3 times, most recently from 7743147 to d671d7b Compare March 16, 2019 15:50
@XVincentX XVincentX force-pushed the feat/condition-refactor branch from d671d7b to b3323bb Compare March 16, 2019 15:54
@XVincentX XVincentX requested a review from DrMegavolt March 16, 2019 16:11
@XVincentX XVincentX force-pushed the feat/condition-refactor branch from d943c59 to 62d9c4a Compare March 16, 2019 16:12
@XVincentX XVincentX force-pushed the feat/condition-refactor branch from 62d9c4a to 1720823 Compare March 16, 2019 16:19
@XVincentX XVincentX force-pushed the feat/condition-refactor branch from 74a2334 to cb37447 Compare March 17, 2019 09:29
@XVincentX XVincentX force-pushed the feat/condition-refactor branch from 801e9ec to 69b39db Compare March 17, 2019 09:59
@XVincentX XVincentX marked this pull request as ready for review March 17, 2019 10:00
@XVincentX XVincentX changed the title Condition engine refactor and improvements Condition engine refactor and improvements — JSON Schema Validation Mar 17, 2019
@DrMegavolt DrMegavolt merged commit 2dbfb7a into master Mar 25, 2019
@DrMegavolt DrMegavolt deleted the feat/condition-refactor branch March 25, 2019 15:49
gatherchou pushed a commit to yilu-tech/express-gateway that referenced this pull request Jul 29, 2021
…xpressGateway#882)

* unify code style with conditions

* show condition name correctly

* refactor pipeline construction

* put default condition in the schema

* provide more data and logger

* restore conditionConfig check

* fix typo

* validate conditions ahead of the time

* temporany compatibility

* check condition presence

* fix test text

* add condition indirection layer

* provide validation for sub conditions

* json schema validator

* add flatmap for node 8 and 10

* use logError to show stuff or not

* Compatibility with old plugins

* move it in conditions

* kill matchEgCondition

* add test for json schema
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.

2 participants