router: Add clusterMetadata RouteEntry interface and implementation. #3003
Conversation
…nvoyproxy#2851 Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
|
Hi. I think this is essentially implementing route local filter configuration. https://github.com/envoyproxy/data-plane-api/issues/540 I suggest two things: renaming this field to “filter_config”, and adding another filter config under virtual host. Then everyone could use this field in their filters for their respective needs. |
|
|
||
| if (cluster.has_cluster_metadata()) { | ||
| cluster_metadata_.MergeFrom(cluster.cluster_metadata()); | ||
| } |
There was a problem hiding this comment.
How will this work when both have same keys?
There was a problem hiding this comment.
The docs for MergeFrom state that 'Singular fields will be overwritten, if specified in from, except for embedded messages which will be merged. Repeated fields will be concatenated'.
Are you asking specifically how MergeFrom works for maps?
|
I think these are slightly different - this metadata is for the routed cluster; and appears on the RouteEntry interface; |
|
IIRC Metadata has a filter_metadata field that is a map from filter name to struct. is that what you mean? |
|
I think it would be good to clarify the intended use of the metadata. If this is purely for consumption via logging/stats, what we have in this PR and the corresponding |
|
the intention is per-weighted cluster additional filter configuration. chiming in to the discussion at envoyproxy/data-plane-api#605 |
|
OK, let's continue the discussion there. I don't think it would be good to have two distinct ways to provide per-route or per-weighted cluster configuration, and we shouldn't call something metadata if it's really configuration, best to call that out as a first class concept. |
|
@mattklein123 just had a chat with @rodaine. It seems that my intention with this PR are a subset of what he is doing. so I can close this PR. |
|
Going to close this per discussion. |
This adds cluster_metadata to route entry so that a filter can get metadata that is specific to the
cluster chosen for routing. A "cluster_metadata" field was added to the RouteAction configuration (see
link to the data plane PR)
Risk Level: Low/Medium
Testing:
Added a unit test to config_impl_test.cc
Docs Changes:
original data pr:
envoyproxy/data-plane-api#598
final pr that unhides docs:
envoyproxy/data-plane-api#607
Release Notes:
envoyproxy/data-plane-api#607
Fixes #2851
API Changes:
envoyproxy/data-plane-api#598