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

test: allow specifying HTTP protocol #822

Merged
merged 4 commits into from
Feb 20, 2024
Merged

test: allow specifying HTTP protocol #822

merged 4 commits into from
Feb 20, 2024

Conversation

spacewander
Copy link
Contributor

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

fixes #730

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

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

Signed-off-by: spacewander <spacewanderlzx@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Feb 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

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

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #822      +/-   ##
==========================================
- Coverage   38.17%   38.10%   -0.07%     
==========================================
  Files          61       61              
  Lines       10412    10428      +16     
==========================================
- Hits         3975     3974       -1     
- Misses       6138     6154      +16     
- Partials      299      300       +1     

see 4 files with indirect coverage changes

@spacewander spacewander marked this pull request as ready for review February 5, 2024 13:55
@sjcsjc123
Copy link
Collaborator

你好,在test/e2e/conformance/tests/go-wasm-sni-misdirect.go文件下有发送http2请求且sni匹配/不匹配的场景,可否帮忙补充一下,插件代码位于plugins/wasm-go/extensions/sni-misdirect/main.go

@@ -152,6 +168,8 @@ func (d *DefaultRoundTripper) CaptureRoundTrip(request Request) (*CapturedReques
}
}

d.initProtocol(client, request.Protocol)
Copy link
Collaborator

@2456868764 2456868764 Feb 7, 2024

Choose a reason for hiding this comment

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

这里代码逻辑没有什么问题,在设置client.Transport时候根据是否http/http2 和 是否TLS请求条件,是否可以重构一下,这样整体代码看起来会清晰舒服一些。

@spacewander
Copy link
Contributor Author

你好,在test/e2e/conformance/tests/go-wasm-sni-misdirect.go文件下有发送http2请求且sni匹配/不匹配的场景,可否帮忙补充一下,插件代码位于plugins/wasm-go/extensions/sni-misdirect/main.go

@sjcsjc123
我试了下,发现 test/e2e/conformance/tests/go-wasm-sni-misdirect.yaml 这个用例里面根本没有配置插件呢。

Signed-off-by: spacewander <spacewanderlzx@gmail.com>
Signed-off-by: spacewander <spacewanderlzx@gmail.com>
@spacewander
Copy link
Contributor Author

你好,在test/e2e/conformance/tests/go-wasm-sni-misdirect.go文件下有发送http2请求且sni匹配/不匹配的场景,可否帮忙补充一下,插件代码位于plugins/wasm-go/extensions/sni-misdirect/main.go

@sjcsjc123 我试了下,发现 test/e2e/conformance/tests/go-wasm-sni-misdirect.yaml 这个用例里面根本没有配置插件呢。

破案了。Higress 的插件默认是全局配置的。原来是这个插件要求 Content-Type 不为空。作为 SNI 相关的插件,这个设计挺神奇的。

@johnlanni
Copy link
Collaborator

@spacewander 这个插件是为了解决这个问题的:#449 所以全局配置。新版本从控制面逻辑解决这个问题了,这个插件作用不大了

@spacewander
Copy link
Contributor Author

@spacewander 这个插件是为了解决这个问题的:#449 所以全局配置。新版本从控制面逻辑解决这个问题了,这个插件作用不大了

原来如此

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 f277d4f into alibaba:main Feb 20, 2024
11 checks passed
Renz7 pushed a commit to Renz7/higress that referenced this pull request Mar 4, 2024
Signed-off-by: spacewander <spacewanderlzx@gmail.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.

e2e支持发送指定http协议的http请求
5 participants