wasm: allow using wasm filter in upstream

[juanmolle]

Commit Message: wasm: allow using wasm filter in upstream

Additional Description: Adds the possibility to use wasm filters as upstream http filter.

Risk Level: Low Testing: yes Docs Changes: yes Release Notes: yes Platform Specific Features: n/a

[repokitteh-read-only[bot]]

As a reminder, PRs marked as draft will not be automatically assigned reviewers, or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

:cat:

Caused by: https://github.com/envoyproxy/envoy/pull/34253 was opened by juanmolle.

see: more, trace.

[repokitteh-read-only[bot]]

CC @envoyproxy/coverage-shephards: FYI only for changes made to (test/per_file_coverage.sh). envoyproxy/coverage-shephards assignee is @alyssawilk

:cat:

Caused by: https://github.com/envoyproxy/envoy/pull/34253 was synchronize by juanmolle.

see: more, trace.

[juanmolle]

@alyssawilk thanks for your review. I revisit the list and:

• downstream functionality: clearRouteCache is used, I check that downstreamCallbacks != null.
• sendLocalReplay: I guess in this item we are safe, the filter keep a flag when a local replay was already sent
• access StreamInfo in non-const: Not sure if it is safe. The filter allows the wasm to access it to store information. I will need some help from a core reviewer here. I could eventually set those access as 'Unimplemented' when the filter is load as upstream. Please let me know if you want me to take that conservative approach.

[juanmolle]

Try to find wasm filter integration test, but I could find them, should I start a new suite?

[alyssawilk]

(pinged @mpwarres internally)

[mpwarres]

Still looking, will finish review tomorrow. Sorry for the delay.

[alyssawilk]

also needs a main merge FYI /wait

[juanmolle]

Apparently it is a legacy flaky test unrelated to this patch

[2024-06-06 13:58:41.381][84597][critical][assert] [contrib/generic_proxy/filters/network/test/integration_test.cc:756] assert failure: waitDownstreamResponseForTest(TestUtility::DefaultTimeout, 2). Details: unexpected timeout
[2024-06-06 13:58:41.381][84597][critical][backtrace] [./source/server/backtrace.h:127] Caught Aborted, suspect faulting address 0x3e900014a75
[2024-06-06 13:58:41.381][84597][critical][backtrace] [./source/server/backtrace.h:111] Backtrace (use tools/stack_decode.py to get line numbers):
[2024-06-06 13:58:41.381][84597][critical][backtrace] [./source/server/backtrace.h:112] Envoy version: 0/1.31.0-dev/test/RELEASE/BoringSSL
[2024-06-06 13:58:41.381][84597][critical][backtrace] [./source/server/backtrace.h:119] #0: __kernel_rt_sigreturn [0xfd109ef939d0]
[2024-06-06 13:58:41.381][84597][critical][backtrace] [./source/server/backtrace.h:119] #1: abort [0xfd109ed08aac]
[2024-06-06 13:58:41.392][84597][critical][backtrace] [./source/server/backtrace.h:119] #2: Envoy::Extensions::NetworkFilters::GenericProxy::(anonymous namespace)::IntegrationTest_MultipleRequestsWithMultipleFrames_Test::TestBody() [0xb3cde0]
[2024-06-06 13:58:41.402][84597][critical][backtrace] [./source/server/backtrace.h:119] #3: testing::internal::HandleExceptionsInMethodIfSupported<>() [0x1ee9274]
[2024-06-06 13:58:41.413][84597][critical][backtrace] [./source/server/backtrace.h:119] #4: testing::Test::Run() [0x1ee9110]
[2024-06-06 13:58:41.423][84597][critical][backtrace] [./source/server/backtrace.h:119] #5: testing::TestInfo::Run() [0x1eea330]
[2024-06-06 13:58:41.434][84597][critical][backtrace] [./source/server/backtrace.h:119] #6: testing::TestSuite::Run() [0x1eeb148]
[2024-06-06 13:58:41.445][84597][critical][backtrace] [./source/server/backtrace.h:119] #7: testing::internal::UnitTestImpl::RunAllTests() [0x1ef9178]
[2024-06-06 13:58:41.455][84597][critical][backtrace] [./source/server/backtrace.h:119] #8: testing::internal::HandleExceptionsInMethodIfSupported<>() [0x1ef8c48]
[2024-06-06 13:58:41.466][84597][critical][backtrace] [./source/server/backtrace.h:119] #9: testing::UnitTest::Run() [0x1ef8aa4]
[2024-06-06 13:58:41.476][84597][critical][backtrace] [./source/server/backtrace.h:119] #10: Envoy::TestRunner::runTests() [0x1894630]
[2024-06-06 13:58:41.487][84597][critical][backtrace] [./source/server/backtrace.h:119] #11: main [0x18933d4]
[2024-06-06 13:58:41.487][84597][critical][backtrace] [./source/server/backtrace.h:119] #12: __libc_start_main [0xfd109ed08e10]
================================================================================
INFO: Elapsed time: 2542.564s, Critical Path: 519.74s
INFO: 37117 processes: 18428 remote cache hit, 13789 internal, 1 local, 4899 processwrapper-sandbox.
//contrib/generic_proxy/filters/network/test:integration_test            FAILED in 11.7s
  /build/bazel_root/base/execroot/envoy/bazel-out/aarch64-opt/testlogs/contrib/generic_proxy/filters/network/test/integration_test/test.log

[juanmolle]

/retest

[alyssawilk]

@mpwarres any further thoughts?

[mpwarres]

Looks really good! WRT @alyssawilk question:

The one think I'm not sure of if there are the stream info setters - it looks like they could safely be called on retries but I'm going to defter to a WASM owner on that one. @mpwarres could you check on that explicitly?

The two places where WasmFilter mutates streamInfo() are in setProperty() and setEnvoyFilterState(). I'm not familiar with what additional restrictions there are on use of streamInfo in upstream filters--is it primarily that in the case of retries, the upstream WasmFilter might run multiple times on a single StreamInfo instance? If so, I think the effect would be that the second invocation would overwrite a FilterState entry written by the first invocation.

Discussed offline with @alyssawilk. In the case of multiple invocations, having the second setProperty() or setEnvoyFilterState() overwrite the value from the first call is fine IMO.

[mpwarres]

@mpwarres any further thoughts?

LGTM