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

New property proposal - is_subrequest #658

Open
dev-null-undefined opened this issue Jan 7, 2025 · 3 comments
Open

New property proposal - is_subrequest #658

dev-null-undefined opened this issue Jan 7, 2025 · 3 comments
Labels
area/proxy-wasm Area: Proxy-Wasm SDK enhancement New feature or request

Comments

@dev-null-undefined
Copy link
Contributor

dev-null-undefined commented Jan 7, 2025

New property named is_subrequest to enable wasm modules to recognize when request get's sliced etc.

Would you be willing to accept merge request adding this property? Or should I leave it just in my fork?

Thanks.

@thibaultcha
Copy link
Member

thibaultcha commented Jan 7, 2025

Hi! Yes, this would be a great addition. The difficult bit is how to namespace the property: request.* is reserved for proxy-wasm compatible properties, wasmx.* is for what we call host properties (set/get properties that would be kong.* in Kong Gateway), and no namespace means the property can be accessed from any phase (outside of request properties). We don't really have a namespace that can receive this property today. Maybe we could define it as ngx.is_subrequest, although the ngx.* namespace is meant for Nginx variable access, and $is_subrequest is not an existing Nginx variable. It could still be our best option though, and reflect the existing OpenResty API users are familiar with.

However to merge this PR we would need test cases (probably 119-proxy_properties_get_ngx.t if we add it to the ngx.* namespace) and documentation, the code only will not be sufficient. For test cases we need positive, negative, and "outside of context" tests to cover all possible cases.

This would be a great addition!

@thibaultcha thibaultcha added enhancement New feature or request area/proxy-wasm Area: Proxy-Wasm SDK labels Jan 7, 2025
@dev-null-undefined dev-null-undefined changed the title New property proposal New property proposal - is_subrequest Jan 7, 2025
@dev-null-undefined
Copy link
Contributor Author

I think putting it under the request.* namespace makes the most sense, I do not like the idea of putting it under the ngx.* namespace as it could lead to some confusion since that is used for nginx variables.

So IMHO I would put it under request.is_subrequest

Or if not there I would introduce new nginx variable named is_subrequest and handle it as a new variable all together.

@thibaultcha
Copy link
Member

thibaultcha commented Jan 8, 2025

Maybe but the issue as I said is that request.* is an Envoy prefix so users writing a filter for this module and using this new property would expect it to work in Envoy as well, but be deceived. And no prefix is a global property. Let's do request.is_subrequest and we'll document it as a host extension.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/proxy-wasm Area: Proxy-Wasm SDK enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants