http: add Kill Request HTTP filter by qqustc · Pull Request #14039 · envoyproxy/envoy

qqustc · 2020-11-16T17:43:02Z

Please review #14170 instead!

repokitteh-read-only · 2020-11-16T17:43:08Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #14039 was opened by qqustc.

see: more, trace.

htuch

Thanks. I'll assume there is no strong objection around general utility and review. I think we should have some CI test to verify this is not included in any normal Envoy binary, or some Bazel magic to make sure it only comes up with a specific --define.
/wait

htuch · 2020-11-19T01:29:06Z

test/extensions/filters/http/kill_request/kill_request_config_test.cc

You shouldn't need to set a deprecated name for a new extension.

Removed this test.

htuch · 2020-11-19T01:29:52Z

source/extensions/filters/http/kill_request/kill_request_config.cc

See comment below about deprecated names. In fact, all extension names are now effectively deprecated. @kyessenov should we have some way to register without declaring these at all?

Yes, we can just do REGISTER_FACTORY(KillRequestFilterFactory, Server::Configuration::NamedHttpFilterConfigFactory)

envoy/include/envoy/registry/registry.h

Line 504 in 9ccd778

* Example registration: REGISTER_FACTORY(SpecificFactory, BaseFactory);

Yeah, the name argument here is supposed to be an alias I think. Factories have unique names with aliases.

htuch · 2020-11-19T01:33:40Z

source/extensions/filters/http/kill_request/kill_request_filter.cc

See existing utilities for this, ProtobufPercentHelper::evaluateFractionalPercent etc.

Thanks Harvey, switched to evaluateFractionalPercent

oschaaf · 2020-11-19T15:17:29Z

docs/root/configuration/http/http_filters/kill_request_filter.rst

drive by comment: what does blast radius mean in this context?

Thanks Otto. Here the blast radius means when we send a kill request to 1 Envoy among a pool of Envoys, how many neighboring Envoy will be affected. This is specific to the downstream use case, so will remove this from the doc.

qqustc · 2020-11-24T04:58:03Z

Thanks. I'll assume there is no strong objection around general utility and review. I think we should have some CI test to verify this is not included in any normal Envoy binary, or some Bazel magic to make sure it only comes up with a specific --define.
/wait

Thank you Harvey! I have commented out the new filter in extensions_build_config.bzl and added a test in main_common_test.cc to verify the filter is not built into Envoy by default.

qqustc · 2020-11-24T15:33:07Z

Thanks. I'll assume there is no strong objection around general utility and review. I think we should have some CI test to verify this is not included in any normal Envoy binary, or some Bazel magic to make sure it only comes up with a specific --define.
/wait

Thank you Harvey! I have commented out the new filter in extensions_build_config.bzl and added a test in main_common_test.cc to verify the filter is not built into Envoy by default.

It turns out CI Generate docs failed after we commented out the new filter in extensions_build_config.bzl because it can't generate the rst doc for kill_request.proto
So instead we modify the definition of envoy_all_extensions() and envoy_all_http_filters() in all_extensions.bzl which is equivalent to commenting out the new filter in extensions_build_config.bzl.

…3721) There are a few limitations in our existing support for symlink-based key rotation: We don't atomically resolve symlinks, so a single snapshot might have inconsistent symlink resolutions for different watched files. Watches are on parent directories, e.g. for /foo/bar/baz on /foo/bar, which doesn't support common key rotation schemes were /foo/new/baz is rotated via a mv -Tf /foo/new /foo/bar. The solution is to provide a structured WatchedDirectory for Secrets to opt into when monitoring DataSources. SDS will used WatchedDirectory to setup the inotify watch instead of the DataSource path. On update, it will read key/cert twice, verifying file content hash consistency. Risk level: Low (opt-in feature) Testing: Unit and integration tests added. Fixes envoyproxy#13663 Fixes envoyproxy#10979 Fixes envoyproxy#13370 Signed-off-by: Harvey Tuch <htuch@google.com> Signed-off-by: Qin Qin <qqin@google.com>

Signed-off-by: Yuchen Dai <silentdai@gmail.com> Signed-off-by: Qin Qin <qqin@google.com>

Signed-off-by: Ryan Northey <ryan@synca.io> Signed-off-by: Qin Qin <qqin@google.com>

Signed-off-by: Piotr Sikora <piotrsikora@google.com> Signed-off-by: Qin Qin <qqin@google.com>

…y doesn't match (envoyproxy#14023) Fixes envoyproxy#14022 Signed-off-by: Dongfang Qu <qudongfang@gmail.com> Signed-off-by: Qin Qin <qqin@google.com>

Signed-off-by: Qin Qin <qqin@google.com>

/bin/bash: line 1: q: command not found Signed-off-by: Qin Qin <qqin@google.com>

Signed-off-by: Qin Qin <qqin@google.com>

…#14131) This fixes a regression which resulted in the downstreamRemoteAddress on the StreamInfo for a connection not having the address supplied by the proxy protocol filter, but instead having the address of the directly connected peer. This issue does not affect HTTP filters. Fixes envoyproxy#14087 Signed-off-by: Greg Greenway <ggreenway@apple.com> Signed-off-by: Qin Qin <qqin@google.com>

… is marked dead by Lua GC (envoyproxy#14092) Fixes envoyproxy#14091 The problem and fix is similiar to envoyproxy#4312 Risk Level: Low Testing: regression test, manual testing Docs Changes: N/A Release Notes: N/A Signed-off-by: Marcin Falkowski <marcin.falkowski@allegro.pl> Signed-off-by: Qin Qin <qqin@google.com>

Swapped test flag use in compile time options to refer to legacy codecs. By default, we use new codecs unless the flag is set FOR legacy. Signed-off-by: Asra Ali <asraa@google.com> Signed-off-by: Qin Qin <qqin@google.com>

Commit Message: Additional Description: Risk Level: Low Testing: unit test, integration, manual testing Docs Changes: Added documentation on how to configure Envoy for tunneling TCP over HTTP/1 Release Notes: n/a (still hidden) Part of envoyproxy#11308 Signed-off-by: Iacopo Rozzo <iacopo@kubermatic.com> Signed-off-by: Qin Qin <qqin@google.com>

…3915) Add extra test case to guard that cluster upstream extension support header rewrite. Signed-off-by: Yuchen Dai <silentdai@gmail.com> Signed-off-by: Qin Qin <qqin@google.com>

envoyproxy#10526 allowed tracers to use CDS clusters. But zipkin proto doc still says bootstrap cluster is mandatory Risk Level: Low Testing: N/A Signed-off-by: Rama Chavali <rama.rao@salesforce.com> Signed-off-by: Qin Qin <qqin@google.com>

Updated link of "life of a request" in STYLE.md. Currently redirecting to 404. Fixes envoyproxy#13397 Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com> Signed-off-by: Qin Qin <qqin@google.com>

…ements (envoyproxy#13960) Signed-off-by: Dmitry Rozhkov <dmitry.rozhkov@intel.com> Signed-off-by: Qin Qin <qqin@google.com>

Add a method to the ThreadLocalOverloadState to support creating arbitrary scaled timers. This can be used by extension filters to scale their behavior in response to load without requiring a separate overload action registration. Filter factories can obtain a reference to the OverloadManager and use that to pass references to the thread-local state into the constructed filters, which can in turn be used to create scaled timers. Signed-off-by: Alex Konradi <akonradi@google.com> Signed-off-by: Qin Qin <qqin@google.com>

Fix a test-only use-after-free that causes flakes of //test/integration:tcp_tunneling_integration_test. Signed-off-by: Alex Konradi <akonradi@google.com> Signed-off-by: Qin Qin <qqin@google.com>

…14140) Signed-off-by: Joshua Marantz <jmarantz@google.com> Signed-off-by: Qin Qin <qqin@google.com>

Use simulated time for the integration test to ensure that timers don't time out due to the test running slow. This prevents test flakes, and TSAN errors when running in that mode. Signed-off-by: Alex Konradi <akonradi@google.com> Signed-off-by: Qin Qin <qqin@google.com>

Right now, the shellcheck format checks all files under the directory, which is unfriendly for a number of reasons (git worktrees, dev scripts, autogenerated code). Instead of using find, use `git ls-files` to list files which can then be filtered. Signed-off-by: Alex Konradi <akonradi@google.com> Signed-off-by: Qin Qin <qqin@google.com>

Abseil thread annotation macros are now prefixed by `ABSL_`. There is no semantic change; this is just a rename. Signed-off-by: Teju Nareddy <nareddyt@google.com> Signed-off-by: Qin Qin <qqin@google.com>

…er (envoyproxy#13797) Signed-off-by: Kevin Baichoo <kbaichoo@google.com> Signed-off-by: Qin Qin <qqin@google.com>

…#14100) * add upstream flood tests to detect empty data and empty continuation frames Signed-off-by: Asra Ali <asraa@google.com> Signed-off-by: Qin Qin <qqin@google.com>

repokitteh-read-only · 2020-11-24T20:43:17Z

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).

🐱

Caused by: #14039 was synchronize by qqustc.

see: more, trace.

qqustc · 2020-11-24T20:48:09Z

Sorry for the spam, accidentally messed up this PR, closed this PR and will open a new one

repokitteh-read-only bot added the api label Nov 16, 2020

qqustc force-pushed the kr2 branch from dcf7a8f to 98f30ab Compare November 16, 2020 17:43

qqustc changed the title ~~api: Add KillRequest proto to configure KillRequest HTTP filter~~ http: add Kill Request HTTP filter Nov 18, 2020

mattklein123 assigned htuch Nov 18, 2020

htuch suggested changes Nov 19, 2020

View reviewed changes

repokitteh-read-only bot added the waiting label Nov 19, 2020

oschaaf reviewed Nov 19, 2020

View reviewed changes

repokitteh-read-only bot removed the waiting label Nov 19, 2020

qqustc marked this pull request as ready for review November 24, 2020 15:34

qqustc requested a review from htuch November 24, 2020 15:38

htuch and others added 17 commits November 24, 2020 15:39

CDS: remove warming cluster if CDS response desired (envoyproxy#13997)

b4f444d

Signed-off-by: Yuchen Dai <silentdai@gmail.com> Signed-off-by: Qin Qin <qqin@google.com>

docs: Bump sphinxext-rediraffe version (envoyproxy#13996)

76f4935

Signed-off-by: Ryan Northey <ryan@synca.io> Signed-off-by: Qin Qin <qqin@google.com>

wasm: fix CPE for Wasmtime. (envoyproxy#14024)

a3fa311

Signed-off-by: Piotr Sikora <piotrsikora@google.com> Signed-off-by: Qin Qin <qqin@google.com>

log the internal error message from *SSL when the cert and private ke…

9b14158

…y doesn't match (envoyproxy#14023) Fixes envoyproxy#14022 Signed-off-by: Dongfang Qu <qudongfang@gmail.com> Signed-off-by: Qin Qin <qqin@google.com>

Add KillRequest proto

8b9dde3

Signed-off-by: Qin Qin <qqin@google.com>

fix format

bc14e00

Signed-off-by: Qin Qin <qqin@google.com>

/bin/bash: line 1: q: command not found

a66d28e

/bin/bash: line 1: q: command not found Signed-off-by: Qin Qin <qqin@google.com>

fix comment

5c5c69a

Signed-off-by: Qin Qin <qqin@google.com>

fix api/versioning/BUILD

ef560b0

Signed-off-by: Qin Qin <qqin@google.com>

fix format

66bdbab

Signed-off-by: Qin Qin <qqin@google.com>

fix format

2f9ebc2

Signed-off-by: Qin Qin <qqin@google.com>

fix dco

e573327

add filter

d39474d

Signed-off-by: Qin Qin <qqin@google.com>

fix format

848f91f

Signed-off-by: Qin Qin <qqin@google.com>

add doc

ba4b8bf

Signed-off-by: Qin Qin <qqin@google.com>

fix comment

018853a

Signed-off-by: Qin Qin <qqin@google.com>

ggreenway and others added 16 commits November 24, 2020 15:41

fix default codec in integration tests (envoyproxy#14101)

44ab44c

Swapped test flag use in compile time options to refer to legacy codecs. By default, we use new codecs unless the flag is set FOR legacy. Signed-off-by: Asra Ali <asraa@google.com> Signed-off-by: Qin Qin <qqin@google.com>

extension: add per host cluster upstream extension test (envoyproxy#1…

cc813eb

…3915) Add extra test case to guard that cluster upstream extension support header rewrite. Signed-off-by: Yuchen Dai <silentdai@gmail.com> Signed-off-by: Qin Qin <qqin@google.com>

Update link of "life of a request" (envoyproxy#14097)

b26692e

Updated link of "life of a request" in STYLE.md. Currently redirecting to 404. Fixes envoyproxy#13397 Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com> Signed-off-by: Qin Qin <qqin@google.com>

perf: add annotations for non-concurrent cross-scoped duration measur…

c6bc86a

…ements (envoyproxy#13960) Signed-off-by: Dmitry Rozhkov <dmitry.rozhkov@intel.com> Signed-off-by: Qin Qin <qqin@google.com>

test: fix use-after-free in fake upstream (envoyproxy#14067)

35a46c8

Fix a test-only use-after-free that causes flakes of //test/integration:tcp_tunneling_integration_test. Signed-off-by: Alex Konradi <akonradi@google.com> Signed-off-by: Qin Qin <qqin@google.com>

coverage: reset expectations for source/common/event/... (envoyproxy#…

a1c3433

…14140) Signed-off-by: Joshua Marantz <jmarantz@google.com> Signed-off-by: Qin Qin <qqin@google.com>

cleanup: Replace deprecated thread annotations macros (envoyproxy#14150)

3a7680e

Abseil thread annotation macros are now prefixed by `ABSL_`. There is no semantic change; this is just a rename. Signed-off-by: Teju Nareddy <nareddyt@google.com> Signed-off-by: Qin Qin <qqin@google.com>

utilities: Implemented an ostream that writes to a user provided buff…

c0f5cff

…er (envoyproxy#13797) Signed-off-by: Kevin Baichoo <kbaichoo@google.com> Signed-off-by: Qin Qin <qqin@google.com>

[test] Add emptydata/continuation to upstream flood tests (envoyproxy…

2616bcd

…#14100) * add upstream flood tests to detect empty data and empty continuation frames Signed-off-by: Asra Ali <asraa@google.com> Signed-off-by: Qin Qin <qqin@google.com>

qqustc force-pushed the kr2 branch from 6aa49e5 to 2616bcd Compare November 24, 2020 20:43

qqustc requested review from PiotrSikora, alyssawilk, asraa, dio, ggreenway, jmarantz, lizan and mattklein123 as code owners November 24, 2020 20:43

repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Nov 24, 2020

qqustc closed this Nov 24, 2020

qqustc mentioned this pull request Nov 24, 2020

http: add Kill Request HTTP filter #14170

Merged

Conversation

qqustc commented Nov 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

repokitteh-read-only bot commented Nov 16, 2020

Uh oh!

htuch left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qqustc commented Nov 24, 2020

Uh oh!

qqustc commented Nov 24, 2020

Uh oh!

repokitteh-read-only bot commented Nov 24, 2020

Uh oh!

qqustc commented Nov 24, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

qqustc commented Nov 16, 2020 •

edited

Loading