-
Notifications
You must be signed in to change notification settings - Fork 478
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: implement custom-response plugin in the golang version #689
Conversation
cc @WeixinX |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
逻辑不符合预期,请再仔细对照并参考 CPP 的实现~
return nil | ||
} | ||
|
||
func onHttpResponseHeaders(ctx wrapper.HttpContext, config CustomResponseConfig, log wrapper.Log) types.Action { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
当插件用于 Mock 响应(enableOnStatus 为空)时,可以在 onHttpRequestHeaders 里使用 proxywasm.SendHttpResponse 自定义响应;若用于修改响应(enableOnStatus 非空)时,在 onHttpRequestHeaders 里返回 types.ActionContinue 即可。参考 CPP:
FilterHeadersStatus PluginRootContext::onRequest(
const CustomResponseConfigRule& rule) {
if (!rule.enable_on_status.empty()) {
return FilterHeadersStatus::Continue;
}
sendLocalResponse(rule.status_code, "", rule.body, rule.headers);
return FilterHeadersStatus::StopIteration;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
而对于修改响应(enableOnStatus 非空)这种用例,需要在 onHttpResponseHeaders 进行处理,即通过 proxywasm.GetHttpResponseHeader(":status") 获取状态码与 enableOnStatus 列表中的状态码匹配,若命中,则返回自定义响应信息,否则直接返回 types.ActionContinue。参考 CPP:
FilterHeadersStatus PluginRootContext::onResponse(
const CustomResponseConfigRule& rule) {
GET_RESPONSE_HEADER_VIEW(":status", status_code);
bool hit = false;
for (const auto& status : rule.enable_on_status) {
if (status_code == status) {
hit = true;
break;
}
}
if (!hit) {
return FilterHeadersStatus::Continue;
}
replaceResponseHeader(Wasm::Common::Http::Header::ContentType,
rule.content_type);
sendLocalResponse(rule.status_code, "", rule.body, rule.headers);
return FilterHeadersStatus::StopIteration;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@WeixinX 已根据建议完成了所有的修改,请重新review,谢谢。
另外我看到 Build and Test Plugins / higress-wasmplugin-test (GO) (pull_request)
测试失败了,但是我在本地跑的时候是成功的。
PLUGIN_TYPE=GO make higress-wasmplugin-test
......
--- PASS: TestHigressConformanceTests (50.32s)
--- SKIP: TestHigressConformanceTests/CPPWasmPluginsBasicAuth (0.00s)
--- SKIP: TestHigressConformanceTests/CPPWasmPluginsKeyAuth (0.00s)
--- SKIP: TestHigressConformanceTests/CPPWasmPluginsRequestBlock (0.00s)
--- PASS: TestHigressConformanceTests/WasmPluginsBasicAuth (1.08s)
--- PASS: TestHigressConformanceTests/WasmPluginsBasicAuth/WasmPlugins_basic-auth (1.05s)
--- PASS: TestHigressConformanceTests/WasmPluginsCustomResponse (1.06s)
--- PASS: TestHigressConformanceTests/WasmPluginsCustomResponse/WasmPlugins_custom-response (1.04s)
--- PASS: TestHigressConformanceTests/WasmPluginsJwtAuth (1.03s)
--- PASS: TestHigressConformanceTests/WasmPluginsJwtAuth/WasmPlugins_jwt-auth (1.01s)
--- PASS: TestHigressConformanceTests/WasmPluginsRequestBlock (1.04s)
--- PASS: TestHigressConformanceTests/WasmPluginsRequestBlock/WasmPlugins_request-block (1.03s)
--- PASS: TestHigressConformanceTests/WasmPluginTransformer (1.06s)
--- PASS: TestHigressConformanceTests/WasmPluginTransformer/WasmPlugin_transformer (1.03s)
--- SKIP: TestHigressConformanceTests/HTTPRouteAppRoot (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteCanaryHeaderWithCustomizedHeader (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteCanaryHeader (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteCanaryWeight (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteConsulHttpBin (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteDefaultBackend (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteDownstreamEncryption (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteEnableCors (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteEnableIgnoreCase (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteEurekaRegistry (0.00s)
--- SKIP: TestHigressConformanceTests/HttpForceRedirectHttps (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteFullPathRegex (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteHostNameSameNamespace (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteHttp2Rpc (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteMatchHeaders (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteMatchMethods (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteMatchPath (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteMatchQueryParams (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRoutePermanentRedirectCode (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRoutePermanentRedirect (0.00s)
--- SKIP: TestHigressConformanceTests/HttpRedirectAsHttps (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteRequestHeaderControl (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteRewriteHost (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteRewritePath (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteSameHostAndPath (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteSimpleSameNamespace (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteTemporalRedirect (0.00s)
--- SKIP: TestHigressConformanceTests/HTTPRouteWhitelistSourceRange (0.00s)
PASS
ok command-line-arguments 50.937s
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
测试失败是因为 tinygo 编译报错
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
编译问题还没来得及看,应该是镜像的系统兼容问题导致的
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #689 +/- ##
==========================================
- Coverage 38.17% 38.13% -0.04%
==========================================
Files 61 61
Lines 10412 10428 +16
==========================================
+ Hits 3975 3977 +2
- Misses 6138 6152 +14
Partials 299 299 |
@WeixinX Hello, 问一下这个PR目前还有什么需要修改的地方吗? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
94ba2e1
to
a6ae6ec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Ⅰ. Describe what this PR did
Implement custom-response plugin in the golang version.
Ⅱ. Does this pull request fix one issue?
fixes #592
Ⅲ. E2E test