-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
api: remove support for cluster_names field in ApiConfigSource protos of type gRPC #3808
api: remove support for cluster_names field in ApiConfigSource protos of type gRPC #3808
Conversation
…ype gRPC. Signed-off-by: James Buckland <jbuckland@google.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, one tiny nit.
source/common/config/utility.cc
Outdated
@@ -90,12 +94,8 @@ void Utility::checkApiConfigSourceNames( | |||
|
|||
if (is_grpc) { | |||
if (api_config_source.cluster_names().size() != 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: !api_config_source.cluster_names().empty()
.
Signed-off-by: James Buckland <jbuckland@google.com>
Signed-off-by: James Buckland <jbuckland@google.com>
Hello guys, do you have some plan to update the documentation? The dynamic_resources:
lds_config:
api_config_source:
api_type: GRPC
cluster_names: [xds_cluster]
cds_config:
api_config_source:
api_type: GRPC
cluster_names: [xds_cluster] dynamic_resources:
lds_config:
api_config_source:
api_type: GRPC
cds_config:
api_config_source:
api_type: GRPC |
@ambuc ^^ |
Signed-off-by: James Buckland jbuckland@google.com
Description:
As noted in #3716, we want to deprecate support for the cluster_names field in ApiConfigSource protos (
api/envoy/api/v2/core/config_source.proto
) of type gRPC. This change only effects configs of type gRPC; configs of type REST and REST_LEGACY are unchanged. This change had its groundwork laid in issue #2860 and PR #3067.Risk Level: Medium. This will throw an EnvoyException on nonconforming configs. This change is part of the 1.8.0 milestone.
Testing:
//test/common/config/utility_test.cc
had to be changed to account for this.Docs Changes: I don't think
docs/root/configuration/overview/v2_overview.rst
needs to be changed for this.Fixes #3716