filter: add network filters to the upstreams #7503

kyessenov · 2019-07-09T04:47:32Z

Description:
Resurrection of PR #6173.
Introduces network filters bound to the upstream clusters.

Risk Level: low
Testing: unit tests
Docs Changes: see #6173
Release Notes: add upstream network filters

Allow network filters to be installed via cluster configuration, uses the same configuration syntax as listener filters. Added the following: - Cluster filter config: api/envoy/api/v2/cluster/filter.proto - Test to verify filters are applied: test/common/upstream/cluster_manager_impl_test.cc - Create filter factory and apply to new upstream connnections - Implement Server::Configuration::FactoryContext using contexts from TransportSocketFactoryContext NEEDS MINOR WORK: Some FactoryContext functions throw NOT_IMPLEMENTED, because some context objects are not obviously available to the upstream code. Needs attention from someone better versed in the architecture, see TODO(aconway) This does not prevent use of the feature, most contexts are available. This feature is used by project: https://github.com/alanconway/envoy-amqp. Signed-off-by: Alan Conway <[email protected]>

Signed-off-by: Kuat Yessenov <[email protected]>

source/common/upstream/upstream_impl.cc

mattklein123

Thanks for getting this going again. This is definitely the right direction, but a few high level comments to get started:

We will needs docs, release notes, etc.
I would like to see an integration test for this with a fake filter.

Thank you!

/wait

source/common/upstream/upstream_impl.cc

Signed-off-by: Kuat Yessenov <[email protected]>

kyessenov · 2019-07-12T19:24:38Z

added an integration test
fixed docs CI failure (hopefully)

Signed-off-by: Kuat Yessenov <[email protected]>

mattklein123

Thanks this is awesome. Some small comments.

/wait

docs/root/intro/version_history.rst

include/envoy/server/filter_config.h

source/common/upstream/upstream_impl.cc

mattklein123 · 2019-07-25T20:35:07Z

source/common/upstream/upstream_impl.cc

+  for (ssize_t i = 0; i < filters.size(); i++) {
+    const auto& proto_config = filters[i];
+    const std::string& string_name = proto_config.name();
+    ENVOY_LOG(debug, "  upstream filter #{}:", i);


What does this debug output look like? It's printed for every cluster so just want to make sure it's useful.

Here is a sample:

[2019-07-25 22:02:21.833][26][info][config] [source/server/configuration_impl.cc:68] loading 1 cluster(s) [2019-07-25 22:02:21.833][27][debug][grpc] [source/common/grpc/google_async_client_impl.cc:43] completionThread running [2019-07-25 22:02:21.839][26][debug][upstream] [source/common/upstream/upstream_impl.cc:695] upstream filter #0: [2019-07-25 22:02:21.839][26][debug][upstream] [source/common/upstream/upstream_impl.cc:696] name: envoy.upstream.polite [2019-07-25 22:02:21.839][26][debug][upstream] [source/common/upstream/cluster_manager_impl.cc:838] adding TLS initial cluster cluster_0 [2019-07-25 22:02:21.840][26][debug][upstream] [source/common/upstream/upstream_impl.cc:845] initializing secondary cluster cluster_0 completed

source/common/upstream/upstream_impl.h

test/common/upstream/BUILD

test/common/upstream/cluster_manager_impl_test.cc

test/integration/cluster_filter_integration_test.cc

Signed-off-by: Kuat Yessenov <[email protected]>

kyessenov · 2019-07-25T22:04:27Z

Thanks for the detailed review! Updated the PR to take the feedback into account.

Signed-off-by: Kuat Yessenov <[email protected]>

mattklein123

Nice, great work. Just 1 small test request.

/wait

source/common/upstream/upstream_impl.h

test/integration/cluster_filter_integration_test.cc

Signed-off-by: Kuat Yessenov <[email protected]>

mattklein123

Looks great with one tiny nit. Thanks!

/wait

mattklein123 · 2019-07-26T22:49:33Z

test/integration/cluster_filter_integration_test.cc

@@ -44,6 +46,7 @@ class PoliteFilter : public Network::Filter, Logger::Loggable<Logger::Id::filter
  }

 private:
+  std::string greeting;


nit: const std::string greeting_;

Sorry you are missing the trailing underscore.

/wait

Duh, thanks.

Signed-off-by: Kuat Yessenov <[email protected]>

mattklein123

Thanks!

alanconway and others added 5 commits March 5, 2019 10:41

merge fix

33a37b3

Signed-off-by: Kuat Yessenov <[email protected]>

format fix

e599e54

Signed-off-by: Kuat Yessenov <[email protected]>

fix breakages

b04f2d8

Signed-off-by: Kuat Yessenov <[email protected]>

fix merge failure

6764c12

Signed-off-by: Kuat Yessenov <[email protected]>

alyssawilk assigned mattklein123 and rshriram Jul 9, 2019

alyssawilk added the waiting label Jul 9, 2019

mattklein123 mentioned this pull request Jul 9, 2019

filters: install network filters on upstream connections #173 #6173

Closed

update to new style

f2dd752

Signed-off-by: Kuat Yessenov <[email protected]>

repokitteh-read-only bot removed the waiting label Jul 9, 2019

kyessenov added 2 commits July 9, 2019 12:16

fix API build

3a8fc53

Signed-off-by: Kuat Yessenov <[email protected]>

link to docs

29d38c0

Signed-off-by: Kuat Yessenov <[email protected]>

kyessenov commented Jul 9, 2019

View reviewed changes

source/common/upstream/upstream_impl.cc Show resolved Hide resolved

lambdai reviewed Jul 10, 2019

View reviewed changes

source/common/upstream/upstream_impl.cc Show resolved Hide resolved

source/common/upstream/upstream_impl.cc Show resolved Hide resolved

mattklein123 requested changes Jul 10, 2019

View reviewed changes

source/common/upstream/upstream_impl.cc Show resolved Hide resolved

repokitteh-read-only bot added the waiting label Jul 10, 2019

kyessenov added 2 commits July 11, 2019 10:52

Merge remote-tracking branch 'upstream/master' into upstream-filter

fd3e05e

add integration test

94387ab

Signed-off-by: Kuat Yessenov <[email protected]>

repokitteh-read-only bot removed the waiting label Jul 11, 2019

kyessenov added 2 commits July 12, 2019 10:22

Merge remote-tracking branch 'upstream/master' into upstream-filter

051a52e

update docs and tests

6a363bd

Signed-off-by: Kuat Yessenov <[email protected]>

lambdai mentioned this pull request Jul 12, 2019

Network::FilterChain: derived interfaces for upstream and downstream #7560

Closed

trying to fix docs

a3c5ae2

Signed-off-by: Kuat Yessenov <[email protected]>

mandarjog mentioned this pull request Jul 15, 2019

Feature Request: Exchange arbitrary metadata between sidecars at connection time #6532

Closed

kyessenov added 3 commits July 16, 2019 11:34

Merge remote-tracking branch 'upstream/master' into upstream-filter

1b4a3ff

add a release note

855b709

Signed-off-by: Kuat Yessenov <[email protected]>

magic number

3b96763

Signed-off-by: Kuat Yessenov <[email protected]>

kyessenov closed this Jul 16, 2019

kyessenov added 2 commits July 24, 2019 13:03

create upstream filter factory context

7a36ddc

Signed-off-by: Kuat Yessenov <[email protected]>

Merge remote-tracking branch 'upstream/master' into upstream-filter

dc048de

repokitteh-read-only bot removed the waiting label Jul 24, 2019

write filter makes more sense for upstream

a654705

Signed-off-by: Kuat Yessenov <[email protected]>

mattklein123 added the waiting label Jul 24, 2019

kyessenov added 2 commits July 24, 2019 23:27

bump-up stat number

236d31d

Signed-off-by: Kuat Yessenov <[email protected]>

clang-tidy

5d8de6e

Signed-off-by: Kuat Yessenov <[email protected]>

repokitteh-read-only bot removed the waiting label Jul 25, 2019

mattklein123 requested changes Jul 25, 2019

View reviewed changes

repokitteh-read-only bot added the waiting label Jul 25, 2019

review feedback

bea06ed

Signed-off-by: Kuat Yessenov <[email protected]>

repokitteh-read-only bot removed the waiting label Jul 25, 2019

mattklein123 added the waiting label Jul 25, 2019

bump up stats integration test

ca2ed10

Signed-off-by: Kuat Yessenov <[email protected]>

repokitteh-read-only bot removed the waiting label Jul 26, 2019

mattklein123 requested changes Jul 26, 2019

View reviewed changes

source/common/upstream/upstream_impl.h Outdated Show resolved Hide resolved

test/integration/cluster_filter_integration_test.cc Outdated Show resolved Hide resolved

repokitteh-read-only bot added the waiting label Jul 26, 2019

review

1a88776

Signed-off-by: Kuat Yessenov <[email protected]>

repokitteh-read-only bot removed the waiting label Jul 26, 2019

mattklein123 requested changes Jul 26, 2019

View reviewed changes

repokitteh-read-only bot added the waiting label Jul 26, 2019

minor nit

2d533b9

Signed-off-by: Kuat Yessenov <[email protected]>

repokitteh-read-only bot added waiting and removed waiting labels Jul 26, 2019

nit

34de48a

Signed-off-by: Kuat Yessenov <[email protected]>

repokitteh-read-only bot removed the waiting label Jul 26, 2019

mattklein123 approved these changes Jul 28, 2019

View reviewed changes

mattklein123 merged commit 0f892c2 into envoyproxy:master Jul 28, 2019

rosskukulinski mentioned this pull request Aug 19, 2019

Add upstream support for PROXYv2 projectcontour/contour#902

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

filter: add network filters to the upstreams #7503

filter: add network filters to the upstreams #7503

kyessenov commented Jul 9, 2019

mattklein123 left a comment

kyessenov commented Jul 12, 2019

mattklein123 left a comment

mattklein123 Jul 25, 2019

kyessenov Jul 25, 2019

kyessenov commented Jul 25, 2019

mattklein123 left a comment

mattklein123 left a comment

mattklein123 Jul 26, 2019

mattklein123 Jul 26, 2019

kyessenov Jul 26, 2019

mattklein123 left a comment

filter: add network filters to the upstreams #7503

filter: add network filters to the upstreams #7503

Conversation

kyessenov commented Jul 9, 2019

mattklein123 left a comment

Choose a reason for hiding this comment

kyessenov commented Jul 12, 2019

mattklein123 left a comment

Choose a reason for hiding this comment

mattklein123 Jul 25, 2019

Choose a reason for hiding this comment

kyessenov Jul 25, 2019

Choose a reason for hiding this comment

kyessenov commented Jul 25, 2019

mattklein123 left a comment

Choose a reason for hiding this comment

mattklein123 left a comment

Choose a reason for hiding this comment

mattklein123 Jul 26, 2019

Choose a reason for hiding this comment

mattklein123 Jul 26, 2019

Choose a reason for hiding this comment

kyessenov Jul 26, 2019

Choose a reason for hiding this comment

mattklein123 left a comment

Choose a reason for hiding this comment