header-to-metadata: add support for regex substitutions by rgs1 · Pull Request #11698 · envoyproxy/envoy

rgs1 · 2020-06-22T20:19:00Z

Commit Message:
Currently, the header-to-metadata filter supports adding
a header's value to a metadata key. This extends this to
support performing a regex match & substitution before
the value is added as metadata.

Additional Description:
The use-case we have is extracting parts of a the :path
header and using those as metadata for routing decisions
via the subset LB.

Risk Level: Low
Testing: Unit tests.
Docs Changes: adding in a bit.
Release Notes: added.

Signed-off-by: Raul Gutierrez Segales rgs@pinterest.com

Currently, the header-to-metadata filter supports adding a header's value to a metadata key. This extends this to support performing a regex match & substitution before the value is added as metadata. The use-case we have is extracting parts of a the :path header and using those as metadata for routing decisions via the subset LB. Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

repokitteh-read-only · 2020-06-22T20:19:08Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy[\w/]*/(v1alpha\d?|v1|v2alpha\d?|v2))|(api/envoy/type/(matcher/)?\w+.proto).
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.
CC @envoyproxy/api-watchers: FYI only for changes made to api/.

🐱

Caused by: #11698 was opened by rgs1.

see: more, trace.

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Convert non covered code into asserts (which is really what they were in the first place). Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

rgs1 · 2020-06-23T11:37:10Z

This failure looks unrelated:

2020-06-22T23:58:42.5265994Z [ RUN      ] Protocols/DownstreamProtocolIntegrationTest.ManyRequestHeadersTimeout/IPv4_Http2Downstream_HttpUpstream
2020-06-22T23:58:42.5266622Z recv: [IB_EXPECT_CONTINUATION]
2020-06-22T23:58:42.5267049Z recv: [IB_EXPECT_CONTINUATION]
2020-06-22T23:58:42.5267427Z recv: [IB_EXPECT_CONTINUATION]
2020-06-22T23:58:42.5267835Z recv: [IB_EXPECT_CONTINUATION]
2020-06-22T23:58:42.5268232Z recv: [IB_EXPECT_CONTINUATION]
2020-06-22T23:58:42.5268640Z recv: [IB_EXPECT_CONTINUATION]
2020-06-22T23:58:42.5269064Z recv: [IB_EXPECT_CONTINUATION]
2020-06-22T23:58:42.5269531Z #0 Envoy::SignalAction::sigHandler() [0x447a2c4]
2020-06-22T23:58:42.5269987Z #1 __restore_rt [0x7fdafd7b4890]
2020-06-22T23:58:42.5270521Z #2 Envoy::HttpIntegrationTest::waitForNextUpstreamRequest() [0x1ef09fa]
2020-06-22T23:58:42.5271166Z #3 Envoy::HttpIntegrationTest::sendRequestAndWaitForResponse() [0x1ef08b5]
2020-06-22T23:58:42.5271824Z #4 Envoy::HttpIntegrationTest::testManyRequestHeaders() [0x1f0826f]
2020-06-22T23:58:42.5272562Z #5 Envoy::DownstreamProtocolIntegrationTest_ManyRequestHeadersTimeout_Test::TestBody() [0x1d5ce35]
2020-06-22T23:58:42.5273317Z #6 testing::internal::HandleSehExceptionsInMethodIfSupported<>() [0x5a01726]
2020-06-22T23:58:42.5273991Z #7 testing::internal::HandleExceptionsInMethodIfSupported<>() [0x59eccc1]
2020-06-22T23:58:42.5274565Z #8 testing::Test::Run() [0x59d7292]
2020-06-22T23:58:42.5275024Z #9 testing::TestInfo::Run() [0x59d7d98]
2020-06-22T23:58:42.5275533Z #10 testing::TestSuite::Run() [0x59d8509]
2020-06-22T23:58:42.5276101Z #11 testing::internal::UnitTestImpl::RunAllTests() [0x59e5516]
2020-06-22T23:58:42.5276733Z #12 testing::internal::HandleSehExceptionsInMethodIfSupported<>() [0x5a06ba6]
2020-06-22T23:58:42.5277305Z #13 testing::internal::HandleExceptionsInMethodIfSupported<>() [0x59efb11]
2020-06-22T23:58:42.5277756Z #14 testing::UnitTest::Run() [0x59e4f60]
2020-06-22T23:58:42.5278098Z #15 RUN_ALL_TESTS() [0x3d5bad5]
2020-06-22T23:58:42.5278493Z #16 Envoy::TestRunner::RunTests() [0x3d5b1cd]
2020-06-22T23:58:42.5278823Z #17 main [0x3d59365]
2020-06-22T23:58:42.5279150Z #18 __libc_start_main [0x7fdafd3d2b97]
2020-06-22T23:58:42.5279737Z external/bazel_tools/tools/test/collect_coverage.sh: line 131: 16048 Aborted                 (core dumped) "$@"
2020-06-22T23:58:42.5280314Z + TEST_STATUS=134
2020-06-22T23:58:42.5282341Z + touch /tmp/sandbox_base/bazel-sandbox.1ae695b0f4561d748dedf8e3b8aae43065ee0770ea20fa59e701e738c4aae63c/processwrapper-sandbox/6392/execroot/envoy/bazel-out/k8-fastbuild/testlogs/test/integration/protocol_integration_test/shard_4_of_5/coverage.dat
2020-06-22T23:58:42.5283690Z + [[ 134 -ne 0 ]]
2020-06-22T23:58:42.5284327Z + echo --
2020-06-22T23:58:42.5284871Z --
2020-06-22T23:58:42.5285335Z + echo Coverage runner: Not collecting coverage for failed test.
2020-06-22T23:58:42.5285903Z Coverage runner: Not collecting coverage for failed test.
2020-06-22T23:58:42.5286443Z + echo The following commands failed with status 134
2020-06-22T23:58:42.5286947Z The following commands failed with status 134
2020-06-22T23:58:42.5288947Z + echo /tmp/sandbox_base/bazel-sandbox.1ae695b0f4561d748dedf8e3b8aae43065ee0770ea20fa59e701e738c4aae63c/processwrapper-sandbox/6392/execroot/envoy/bazel-out/k8-fastbuild/bin/test/integration/protocol_integration_test.runfiles/envoy/test/integration/protocol_integration_test --gmock_default_mock_behavior=2 '--log-path /dev/null' '-l trace'
2020-06-22T23:58:42.5291791Z /tmp/sandbox_base/bazel-sandbox.1ae695b0f4561d748dedf8e3b8aae43065ee0770ea20fa59e701e738c4aae63c/processwrapper-sandbox/6392/execroot/envoy/bazel-out/k8-fastbuild/bin/test/integration/protocol_integration_test.runfiles/envoy/test/integration/protocol_integration_test --gmock_default_mock_behavior=2 --log-path /dev/null -l trace
2020-06-22T23:58:42.5293100Z + exit 134
2020-06-22T23:58:42.5293524Z ================================================================================

…a-support-regex

snowp

Looks good for the most part, just some minor comments.

Seems like something funny happened with the proto sync - I don't think it should be touching V2 anymore? @htuch

source/extensions/filters/http/header_to_metadata/header_to_metadata_filter.cc

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

source/extensions/filters/http/header_to_metadata/header_to_metadata_filter.cc

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

htuch

API looks good, just a couple of nits.

api/envoy/extensions/filters/http/header_to_metadata/v3/header_to_metadata.proto

* annotate oneof migration * link to fields that aren't in the same msg Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

repokitteh-read-only · 2020-06-24T14:21:41Z

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: #11698 was synchronize by rgs1.

see: more, trace.

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

snowp

Just some nitty stuff, otherwise LGTM

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

source/extensions/filters/http/header_to_metadata/header_to_metadata_filter.cc

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

source/extensions/filters/http/header_to_metadata/header_to_metadata_filter.cc

Our docs said this wasn't allowed and during runtime we'd skip empty values; let's reject them during config time (so that the new ASSERT() which replaced the empty value check doesn't trigger a crash on debug builds). Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

htuch · 2020-06-24T23:29:58Z

/lgtm api

rgs1 · 2020-06-25T13:49:14Z

Failure is related to admission control integration test:

-----------------------------------------------------------------------------
[==========] Running 4 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 4 tests from IpVersions/AdmissionControlIntegrationTest
[ RUN      ] IpVersions/AdmissionControlIntegrationTest.HttpTest/0
[       OK ] IpVersions/AdmissionControlIntegrationTest.HttpTest/0 (74097 ms)
[ RUN      ] IpVersions/AdmissionControlIntegrationTest.HttpTest/1
[       OK ] IpVersions/AdmissionControlIntegrationTest.HttpTest/1 (74677 ms)
[ RUN      ] IpVersions/AdmissionControlIntegrationTest.GrpcTest/0
[       OK ] IpVersions/AdmissionControlIntegrationTest.GrpcTest/0 (75568 ms)
[ RUN      ] IpVersions/AdmissionControlIntegrationTest.GrpcTest/1
-- Test timed out at 2020-06-25 03:02:16 UTC --
==================
WARNING: ThreadSanitizer: signal-unsafe call inside of a signal (pid=15)
    #0 malloc /home/brian/src/final/llvm-project/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:651:5 (admission_control_integration_test+0x295d564)
    #1 strdup <null> (libc.so.6+0x9d9b9)
    #2 spdlog::details::os::localtime(long const&) /proc/self/cwd/external/com_github_gabime_spdlog/include/spdlog/details/os-inl.h:97:5 (admission_control_integration_test+0x2a5b1e7)
    #3 spdlog::pattern_formatter::get_time_(spdlog::details::log_msg const&) /proc/self/cwd/external/com_github_gabime_spdlog/include/spdlog/details/pattern_formatter-inl.h:1037:16 (admission_control_integration_test+0x2aa0495)
    #4 spdlog::pattern_formatter::format(spdlog::details::log_msg const&, fmt::v6::basic_memory_buffer<char, 250ul, std::__1::allocator<char> >&) /proc/self/cwd/external/com_github_gabime_spdlog/include/spdlog/details/pattern_formatter-inl.h:1021:22 (admission_control_integration_test+0x29f8be1)
    #5 Envoy::Logger::DelegatingLogSink::log(spdlog::details::log_msg const&) /proc/self/cwd/source/common/common/logger.cc:58:17 (admission_control_integration_test+0x7a34241)
    #6 spdlog::logger::sink_it_(spdlog::details::log_msg const&) /proc/self/cwd/external/com_github_gabime_spdlog/include/spdlog/logger-inl.h:178:23 (admission_control_integration_test+0x29f859b)
    #7 void spdlog::logger::log<char const*, void const*>(spdlog::source_loc, spdlog::level::level_enum, fmt::v6::basic_string_view<char>, char const* const&, void const* const&) /proc/self/cwd/external/com_github_gabime_spdlog/include/spdlog/logger.h:92:17 (admission_control_integration_test+0x6010e9b)
    #8 Envoy::BackwardsTrace::logFault(char const*, void const*) /proc/self/cwd/bazel-out/k8-dbg/bin/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:104:5 (admission_control_integration_test+0x6010470)
    #9 Envoy::SignalAction::sigHandler(int, siginfo_t*, void*) /proc/self/cwd/source/common/signal/signal_action.cc:57:10 (admission_control_integration_test+0x600e086)
    #10 __tsan::CallUserSignalHandler(__tsan::ThreadState*, bool, bool, bool, int, __sanitizer::__sanitizer_siginfo*, void*) /home/brian/src/final/llvm-project/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:1907:7 (admission_control_integration_test+0x29656cf)
    #11 void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2433:10 (admission_control_integration_test+0x7b4ad6c)
    #12 void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2469:14 (admission_control_integration_test+0x7b2edfe)
    #13 testing::Test::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2508:5 (admission_control_integration_test+0x7b14121)
    #14 testing::TestInfo::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2684:11 (admission_control_integration_test+0x7b14fc3)
    #15 testing::TestSuite::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2816:28 (admission_control_integration_test+0x7b159fa)
    #16 testing::internal::UnitTestImpl::RunAllTests() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:5338:44 (admission_control_integration_test+0x7b2519d)
    #17 bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2433:10 (admission_control_integration_test+0x7b522ec)
    #18 bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2469:14 (admission_control_integration_test+0x7b32b0e)
    #19 testing::UnitTest::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:4925:10 (admission_control_integration_test+0x7b24a5b)
    #20 RUN_ALL_TESTS() /proc/self/cwd/external/com_google_googletest/googletest/include/gtest/gtest.h:2473:46 (admission_control_integration_test+0x5635ed7)
    #21 Envoy::TestRunner::RunTests(int, char**) /proc/self/cwd/test/test_runner.cc:134:10 (admission_control_integration_test+0x563524d)
    #22 main /proc/self/cwd/test/main.cc:34:10 (admission_control_integration_test+0x5632b91)

SUMMARY: ThreadSanitizer: signal-unsafe call inside of a signal (/lib/x86_64-linux-gnu/libc.so.6+0x9d9b9) in strdup

cc: @tonya11en

…a-support-regex

tonya11en · 2020-06-25T18:51:38Z

I'm looking into the admission control test failures. Thanks for the heads-up.

source/extensions/filters/http/header_to_metadata/header_to_metadata_filter.cc

test/extensions/filters/http/header_to_metadata/config_test.cc

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

zuercher

Thanks!

…a-support-regex

snowp

LGTM, thanks!

snowp · 2020-06-26T16:44:56Z

Needs @envoyproxy/api-shepherds approval

htuch · 2020-06-26T17:39:18Z

/lgtm api

rgs1 requested a review from zuercher as a code owner June 22, 2020 20:19

repokitteh-read-only bot added v2-freeze api labels Jun 22, 2020

Raul Gutierrez Segales added 4 commits June 22, 2020 16:26

docs: add a line & example for this new feature

67b4bf5

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Fix coverage

08d2562

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

More coverage improvement

24cd934

Convert non covered code into asserts (which is really what they were in the first place). Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Avoid copy

d00253b

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Merge remote-tracking branch 'upstream/master' into header-to-metadat…

565f64e

…a-support-regex

asraa assigned zuercher Jun 23, 2020

snowp suggested changes Jun 23, 2020

View reviewed changes

Raul Gutierrez Segales added 6 commits June 23, 2020 12:22

Drop v2 proto changes

e571428

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Simplify config presence check

ae05f17

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Fix doc example

45b1fb1

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Simplify check

61c19c3

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Add comment wrt rewritten_value's purpose.

f96e5bc

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Use emplace_back

44dee89

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

htuch reviewed Jun 24, 2020

View reviewed changes

api/envoy/extensions/filters/http/header_to_metadata/v3/header_to_metadata.proto Outdated Show resolved Hide resolved

api/envoy/extensions/filters/http/header_to_metadata/v3/header_to_metadata.proto Outdated Show resolved Hide resolved

htuch removed the v2-freeze label Jun 24, 2020

config fixes

90969d8

* annotate oneof migration * link to fields that aren't in the same msg Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Fix ref links

62c635f

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

snowp suggested changes Jun 24, 2020

View reviewed changes

docs/root/configuration/http/http_filters/header_to_metadata_filter.rst Outdated Show resolved Hide resolved

source/extensions/filters/http/header_to_metadata/header_to_metadata_filter.cc Show resolved Hide resolved

Raul Gutierrez Segales added 2 commits June 24, 2020 17:38

Fix grammar

c895a89

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Add TODO that'll be addressed once we are on v4

2b3b440

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

zuercher reviewed Jun 24, 2020

View reviewed changes

source/extensions/filters/http/header_to_metadata/header_to_metadata_filter.cc Outdated Show resolved Hide resolved

source/extensions/filters/http/header_to_metadata/header_to_metadata_filter.cc Show resolved Hide resolved

repokitteh-read-only bot removed the api label Jun 24, 2020

Merge remote-tracking branch 'upstream/master' into header-to-metadat…

0e95a5c

…a-support-regex

zuercher reviewed Jun 25, 2020

View reviewed changes

Raul Gutierrez Segales added 4 commits June 25, 2020 17:15

Fix spelling

e4256d7

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Release note about rejecting on_header_missing rules with empty value

9a8f941

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Add missing comments for test cases.

fa43779

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Add metadata regardless of match/substitution result

e0f0cbe

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

repokitteh-read-only bot added the api label Jun 25, 2020

zuercher approved these changes Jun 25, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into header-to-metadat…

bde894e

…a-support-regex

snowp approved these changes Jun 26, 2020

View reviewed changes

rgs1 mentioned this pull request Jun 26, 2020

Fix header-to-metadata if clause #11747

Merged

repokitteh-read-only bot removed the api label Jun 26, 2020

htuch merged commit 5f1348b into envoyproxy:master Jun 26, 2020

rgs1 mentioned this pull request Jun 30, 2020

extend route's hashing policy to support hashing on a regex substitution #11811

Closed

Conversation

rgs1 commented Jun 22, 2020

Uh oh!

repokitteh-read-only bot commented Jun 22, 2020

Uh oh!

rgs1 commented Jun 23, 2020

Uh oh!

snowp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

htuch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

repokitteh-read-only bot commented Jun 24, 2020

Uh oh!

snowp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

htuch commented Jun 24, 2020

Uh oh!

rgs1 commented Jun 25, 2020

Uh oh!

tonya11en commented Jun 25, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zuercher left a comment

Choose a reason for hiding this comment

Uh oh!

snowp left a comment

Choose a reason for hiding this comment

Uh oh!

snowp commented Jun 26, 2020

Uh oh!

htuch commented Jun 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants