docs: remove guidance to use names #21763

kyessenov · 2022-06-17T20:20:48Z

Signed-off-by: Kuat Yessenov [email protected]
Commit Message: make a pass to remove stale suggestion to use filter names.
Risk Level: low
Testing: none
Docs Changes: yes
Release Notes: none
Issue: Fix #21759

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

repokitteh-read-only · 2022-06-17T20:20:51Z

Docs for this Pull Request will be rendered here:

https://storage.googleapis.com/envoy-pr/21763/docs/index.html

The docs are (re-)rendered each time the CI envoy-presubmit (precheck docs) job completes.

🐱

Caused by: #21763 was opened by kyessenov.

see: more, trace.

shashankram

Hey @kyessenov, thanks for improving the docs.

Could you clarify if we need to use wellknown names or just have the name match the filter config on the listener when using typed_per_filter_config which is a map of filter_name->config?
See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-msg-config-route-v3-route, where for typed_per_filter_config it says "The key should match the filter name, such as envoy.filters.http.buffer for the HTTP buffer filter".

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

shashankram · 2022-06-17T21:42:25Z

Hey @kyessenov, thanks for improving the docs.

Could you clarify if we need to use wellknown names or just have the name match the filter config on the listener when using typed_per_filter_config which is a map of filter_name->config? See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-msg-config-route-v3-route, where for typed_per_filter_config it says "The key should match the filter name, such as envoy.filters.http.buffer for the HTTP buffer filter".

This is actually resulting in an issue described in #21759 (comment)

- Updates Envoy to its latest available version (v1.22.2 for Linux, v1.22.1 for Windows). The latest version includes the latest released security fix. We could not update Envoy previously due to a blocking bug: envoyproxy/envoy#20113 - Updates filter names to custom names as wellknown names are deprecated in Envoy (with 1 exception for the http.rbac filter). Envoy will use the TypeURL in the proto to determine which filter to use instead. Wellknown names are not required and using them is confusing because not all filters are defined in the legacy wellknown pkg (e.g. http.local_ratelimit). See: envoyproxy/envoy#21759 envoyproxy/envoy#21763 envoyproxy/go-control-plane#293 envoyproxy/go-control-plane#552 - Uses the distroless image as the alpine image has been discontinued: envoyproxy/envoy#21758 - Updates tests to use custom filter names - Adds `proto_types.go` to aid dynamic proto resolution for typed configs using `any.Any()`. This helps resolve protos where dynamic resolution is necessary. - Updated Prometheus' ConfigMap to reflect changes to Envoy metrics prefixes Signed-off-by: Shashank Ram <[email protected]>

phlax · 2022-06-27T06:58:44Z

not sure what the docs errors are - something about empty empty

https://dev.azure.com/cncf/envoy/_build/results?buildId=110880&view=logs&j=91e633a5-4907-5da1-5862-92a79fe2387a&t=f29e24e7-a093-5724-80bd-d79975d268cb&l=392

phlax · 2022-07-06T07:19:49Z

@kyessenov wondering what the status is on this one - would be good to update the docs before we cut the next release i think

/wait-any

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

kyessenov · 2022-07-06T20:32:17Z

@phlax The empty error in docs was because I removed duplicated pages for filters, see how we have two links for some filters:

kyessenov · 2022-07-08T15:42:19Z

Let's merge this before a release @phlax?

kyessenov · 2022-07-11T23:41:51Z

@phlax Gentle ping.

phlax · 2022-07-12T18:47:30Z

hi - apologies - ive been a bit snowed under - ill review tomorrow

phlax

@kyessenov can we add the new type_url where we are removing the old info ?

many of these pages have incorrect config examples (~unrelatedly) so without the type url they are a bit unhelpful

phlax · 2022-07-13T11:52:41Z

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

@@ -20,8 +20,6 @@ Following are supported JWT alg:
 Configuration
 -------------

-This filter should be configured with the name *envoy.filters.http.jwt_authn*.


this one is missing any api link - which leaves the page a bit of a dead end in terms of configuration

Yes, let's put back the type URLs where it's missing. The link is the type URL that should be used generally.

phlax · 2022-07-13T12:43:10Z

~~it looks like we have a lot of these in config examples too judging from:~~

this was incorrect, the names are still required

$ git grep "\- name: envoy"

~~i will add a pr to clean up all the ones in yaml files - the ones in rst are more problematic i think~~

phlax · 2022-07-13T12:56:55Z

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

@@ -5,7 +5,6 @@ Cache filter

 * :ref:`v3 API reference <envoy_v3_api_msg_extensions.filters.http.cache.v3.CacheConfig>`
 * :ref:`v3 SimpleHTTPCache API reference <envoy_v3_api_msg_extensions.cache.simple_http_cache.v3.SimpleHttpCacheConfig>`
-* This filter should be configured with the name ``envoy.filters.http.cache``.


maybe im not understanding something here

$ envoydev/envoy/examples/cache$ git diff

diff --git a/examples/cache/front-envoy.yaml b/examples/cache/front-envoy.yaml index b953d1e781..caa0ee8b9f 100644 --- a/examples/cache/front-envoy.yaml +++ b/examples/cache/front-envoy.yaml @@ -27,8 +27,7 @@ static_resources: route: cluster: service2 http_filters: - - name: "envoy.filters.http.cache" - typed_config: + - typed_config: "@type": "type.googleapis.com/envoy.extensions.filters.http.cache.v3.CacheConfig" typed_config: "@type": "type.googleapis.com/envoy.extensions.cache.simple_http_cache.v3.SimpleHttpCacheConfig"

$ ./verify.sh ... front-envoy_1 | [2022-07-13 12:50:15.726][9][critical][main] [source/server/server.cc:117] error initializing configuration '/etc/front-envoy.yaml': Proto constraint validation failed (HttpConnectionManagerValidationError.HttpFilters[0]: embedded message failed validation | caused by HttpFilterValidationError.Name: value length must be at least 1 characters): stat_prefix: "ingress_http" ...

ok i see

diff --git a/examples/cache/front-envoy.yaml b/examples/cache/front-envoy.yaml index b953d1e781..36bcbcb27b 100644 --- a/examples/cache/front-envoy.yaml +++ b/examples/cache/front-envoy.yaml @@ -27,7 +27,7 @@ static_resources: route: cluster: service2 http_filters: - - name: "envoy.filters.http.cache" + - name: "my.own.special.filter.name" typed_config: "@type": "type.googleapis.com/envoy.extensions.filters.http.cache.v3.CacheConfig"

...works, hmmm

Yes, name is required. The incorrect statement is that it must be the qualified extension name. It can be anything.

phlax

@kyessenov

how about instead of just removing the name advice we replace with something like:

At a minimum, this filter must be configured with the following @type config and a name of your choosing:

          http_filters:
          - name: "envoy.filters.http.cache"
            typed_config:
              "@type": "type.googleapis.com/envoy.extensions.filters.http.cache.v3.CacheConfig"

I guess we need to be careful not to imply that the config will be sufficient - but if we can think of the right words i think this would be more useful to end users than just removing the ~incorrect advice

phlax · 2022-07-14T09:37:50Z

/wait-any

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

phlax

thnks for resolving this @kyessenov - i think this is a really helpful update to the docs, lgtm

phlax · 2022-07-14T20:28:27Z

/retest

repokitteh-read-only · 2022-07-14T20:28:31Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #21763 (comment) was created by @phlax.

see: more, trace.

docs: remove guidance to use names

906b22c

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

kyessenov requested a review from phlax June 17, 2022 20:20

shashankram reviewed Jun 17, 2022

View reviewed changes

duplicate extensions?

d02123e

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

phlax self-assigned this Jun 20, 2022

shashankram mentioned this pull request Jun 21, 2022

envoy: update to latest version and fix typed proto usage openservicemesh/osm#4834

Merged

repokitteh-read-only bot added the waiting:any label Jul 6, 2022

merge fix

50df656

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

repokitteh-read-only bot removed the waiting:any label Jul 6, 2022

stale links

fa6869a

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

phlax added this to the 1.23.0 milestone Jul 12, 2022

phlax reviewed Jul 13, 2022

View reviewed changes

repokitteh-read-only bot added the waiting:any label Jul 14, 2022

kyessenov added 2 commits July 14, 2022 17:11

Merge remote-tracking branch 'upstream/main' into remove_name_clause

6f47bd7

review

ff6e2b7

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

repokitteh-read-only bot removed the waiting:any label Jul 14, 2022

phlax approved these changes Jul 14, 2022

View reviewed changes

kyessenov merged commit 9b6bbf2 into envoyproxy:main Jul 14, 2022

adisuissa mentioned this pull request Nov 7, 2022

Provide custom responses based on upstream reply using redirection #23084

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: remove guidance to use names #21763

docs: remove guidance to use names #21763

kyessenov commented Jun 17, 2022 •

edited by phlax

Loading

repokitteh-read-only bot commented Jun 17, 2022

shashankram left a comment

shashankram commented Jun 17, 2022

phlax commented Jun 27, 2022 •

edited

Loading

phlax commented Jul 6, 2022

kyessenov commented Jul 6, 2022

kyessenov commented Jul 8, 2022

kyessenov commented Jul 11, 2022

phlax commented Jul 12, 2022

phlax left a comment

phlax Jul 13, 2022

kyessenov Jul 13, 2022

phlax commented Jul 13, 2022 •

edited

Loading

phlax Jul 13, 2022

kyessenov Jul 13, 2022

phlax left a comment •

edited

Loading

phlax commented Jul 14, 2022

phlax left a comment

phlax commented Jul 14, 2022

repokitteh-read-only bot commented Jul 14, 2022

docs: remove guidance to use names #21763

docs: remove guidance to use names #21763

Conversation

kyessenov commented Jun 17, 2022 • edited by phlax Loading

repokitteh-read-only bot commented Jun 17, 2022

shashankram left a comment

Choose a reason for hiding this comment

shashankram commented Jun 17, 2022

phlax commented Jun 27, 2022 • edited Loading

phlax commented Jul 6, 2022

kyessenov commented Jul 6, 2022

kyessenov commented Jul 8, 2022

kyessenov commented Jul 11, 2022

phlax commented Jul 12, 2022

phlax left a comment

Choose a reason for hiding this comment

phlax Jul 13, 2022

Choose a reason for hiding this comment

kyessenov Jul 13, 2022

Choose a reason for hiding this comment

phlax commented Jul 13, 2022 • edited Loading

phlax Jul 13, 2022

Choose a reason for hiding this comment

kyessenov Jul 13, 2022

Choose a reason for hiding this comment

phlax left a comment • edited Loading

Choose a reason for hiding this comment

phlax commented Jul 14, 2022

phlax left a comment

Choose a reason for hiding this comment

phlax commented Jul 14, 2022

repokitteh-read-only bot commented Jul 14, 2022

kyessenov commented Jun 17, 2022 •

edited by phlax

Loading

phlax commented Jun 27, 2022 •

edited

Loading

phlax commented Jul 13, 2022 •

edited

Loading

phlax left a comment •

edited

Loading