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

feat: add request-validation plugin #700

Merged
merged 13 commits into from Feb 2, 2024

Conversation

sjcsjc123
Copy link
Collaborator

Ⅰ. Describe what this PR did

添加request validation的插件

Ⅱ. Does this pull request fix one issue?

fixes: #682

Ⅲ. Why don't you add test cases (unit test/integration test)?

后面补充一下

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

根据个人的理解去改的,没有照搬apisix,类比于apisix添加验证策略方式大致如下:

{
    "body_schema": {
        "filed1": {
          "required": true,
          "type": "string",
          "minLength": 1,
          "maxLength": 10
        },
        "filed2": {
          "required": true,
          "type": "string",
          "minLength": 1,
          "maxLength": 10,
          "pattern": "^[a-zA-Z0-9]+$"
        }
    }
}

每个字段的object类型会映射为各自的策略方式,header写法一样,也可添加校验失败返回的code和message

Signed-off-by: sjcsjc123 <1401189096@qq.com>
@codecov-commenter
Copy link

codecov-commenter commented Dec 16, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a71ecf4) 38.08% compared to head (76af4ca) 38.08%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #700   +/-   ##
=======================================
  Coverage   38.08%   38.08%           
=======================================
  Files          61       61           
  Lines       10323    10323           
=======================================
  Hits         3932     3932           
  Misses       6091     6091           
  Partials      300      300           

sjcsjc123 and others added 2 commits December 18, 2023 12:24
@johnlanni
Copy link
Collaborator

https://github.com/xeipuuv/gojsonschema
可以使用这个lib来做json schema的解析和校验

@sjcsjc123
Copy link
Collaborator Author

https://github.com/xeipuuv/gojsonschema 可以使用这个lib来做json schema的解析和校验

好的

@johnlanni
Copy link
Collaborator

@sjcsjc123 那这个PR我先转为draft了 等实现了json schema的解析校验之后再review下

@johnlanni johnlanni marked this pull request as draft December 25, 2023 05:38
@sjcsjc123
Copy link
Collaborator Author

@johnlanni

由于e2e暂不支持body传参,以及header暂只支持string类型,e2e测试提供的样例并不全面,以下为本地测试简单样例的结果。

采用docker compose的方式来模拟wasm和envoy。

  • 本地header相关的测试结果如下:
    image
  • 本地body相关的测试结果如下:
    image

@sjcsjc123 sjcsjc123 marked this pull request as ready for review December 29, 2023 15:10
@johnlanni
Copy link
Collaborator

@sjcsjc123 建议用petstore的addpet接口来做一下测试:https://petstore.swagger.io/#/pet/addPet

@sjcsjc123
Copy link
Collaborator Author

@sjcsjc123 建议用petstore的addpet接口来做一下测试:https://petstore.swagger.io/#/pet/addPet

我理解的是,为e2e框架添加新的测试板块,去构造一个支持携带body的http request请求,发送给wasm插件进行过滤,然后转发给addpet接口判断能否返回200响应是吗?

@johnlanni
Copy link
Collaborator

@sjcsjc123 e2e框架支持带body测试的工作(#733 )目前正在做,你可以等这个完成再做测试,或者本地先手动测试一下也可以

@sjcsjc123
Copy link
Collaborator Author

好的,我等 #733 完成

@sjcsjc123 sjcsjc123 marked this pull request as draft January 2, 2024 02:39
sjcsjc123 and others added 3 commits January 14, 2024 23:25
Signed-off-by: sjcsjc123 <1401189096@qq.com>
Signed-off-by: sjcsjc123 <1401189096@qq.com>
@sjcsjc123 sjcsjc123 marked this pull request as ready for review January 14, 2024 16:36
@sjcsjc123
Copy link
Collaborator Author

@johnlanni e2e测试已完善body校验

Copy link
Collaborator

@johnlanni johnlanni left a comment

Choose a reason for hiding this comment

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

LGTM

@johnlanni johnlanni merged commit c63cdb6 into alibaba:main Feb 2, 2024
11 checks passed
Renz7 pushed a commit to Renz7/higress that referenced this pull request Mar 4, 2024
Signed-off-by: sjcsjc123 <1401189096@qq.com>
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.

Support APISIX plugin - request-validation
3 participants