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: implement custom-response plugin in the golang version #689

Merged
merged 6 commits into from
Feb 20, 2024

Conversation

cr7258
Copy link
Collaborator

@cr7258 cr7258 commented Dec 13, 2023

Ⅰ. Describe what this PR did

Implement custom-response plugin in the golang version.

Ⅱ. Does this pull request fix one issue?

fixes #592

Ⅲ. E2E test

--- PASS: TestHigressConformanceTests (70.12s)
    --- PASS: TestHigressConformanceTests/WasmPluginsCustomResponse (1.19s)
        --- PASS: TestHigressConformanceTests/WasmPluginsCustomResponse/WasmPlugins_custom-response (1.12s)

@CLAassistant
Copy link

CLAassistant commented Dec 13, 2023

CLA assistant check
All committers have signed the CLA.

@johnlanni
Copy link
Collaborator

cc @WeixinX

Copy link
Collaborator

@WeixinX WeixinX left a comment

Choose a reason for hiding this comment

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

逻辑不符合预期,请再仔细对照并参考 CPP 的实现~

plugins/wasm-go/extensions/custom-response/main.go Outdated Show resolved Hide resolved
plugins/wasm-go/extensions/custom-response/main.go Outdated Show resolved Hide resolved
plugins/wasm-go/extensions/custom-response/main.go Outdated Show resolved Hide resolved
return nil
}

func onHttpResponseHeaders(ctx wrapper.HttpContext, config CustomResponseConfig, log wrapper.Log) types.Action {
Copy link
Collaborator

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;
}

Copy link
Collaborator

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;
}

Copy link
Collaborator Author

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

测试失败是因为 tinygo 编译报错

Copy link
Collaborator

Choose a reason for hiding this comment

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

编译问题还没来得及看,应该是镜像的系统兼容问题导致的

@codecov-commenter
Copy link

codecov-commenter commented Dec 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (612c94d) 38.17% compared to head (9598425) 38.13%.
Report is 16 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            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              

see 3 files with indirect coverage changes

@cr7258 cr7258 requested a review from WeixinX December 27, 2023 02:47
@cr7258 cr7258 requested a review from WeixinX December 28, 2023 03:11
@cr7258 cr7258 requested a review from WeixinX December 28, 2023 08:17
@cr7258
Copy link
Collaborator Author

cr7258 commented Jan 8, 2024

@WeixinX Hello, 问一下这个PR目前还有什么需要修改的地方吗?

Copy link
Collaborator

@WeixinX WeixinX left a comment

Choose a reason for hiding this comment

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

LGTM

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 a138a03 into alibaba:main Feb 20, 2024
11 checks passed
Renz7 pushed a commit to Renz7/higress that referenced this pull request Mar 4, 2024
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.

Implement the Golang version of the existing CPP Wasm plugin: custom-response
5 participants