From eeef8fab5ae9966755e5bc11145a8bc1264f6112 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Tue, 18 Aug 2020 19:39:59 -0400 Subject: [PATCH 1/4] udpa: context parameter encoding. This patch introduces the xDS transport++ context parameter encoding algorithm. Risk level: Low Testing: Unit tests added. Signed-off-by: Harvey Tuch --- api/envoy/config/bootstrap/v3/bootstrap.proto | 25 ++- .../config/bootstrap/v4alpha/bootstrap.proto | 25 ++- .../envoy/config/bootstrap/v3/bootstrap.proto | 25 ++- .../config/bootstrap/v4alpha/bootstrap.proto | 25 ++- source/common/config/BUILD | 12 ++ source/common/config/udpa_context_params.cc | 91 ++++++++++ source/common/config/udpa_context_params.h | 33 ++++ source/common/config/udpa_resource.h | 2 + test/common/config/BUILD | 17 ++ .../common/config/udpa_context_params_test.cc | 156 ++++++++++++++++++ test/common/config/udpa_resource_test.cc | 17 +- test/common/config/udpa_test_utility.h | 16 ++ 12 files changed, 428 insertions(+), 16 deletions(-) create mode 100644 source/common/config/udpa_context_params.cc create mode 100644 source/common/config/udpa_context_params.h create mode 100644 test/common/config/udpa_context_params_test.cc create mode 100644 test/common/config/udpa_test_utility.h diff --git a/api/envoy/config/bootstrap/v3/bootstrap.proto b/api/envoy/config/bootstrap/v3/bootstrap.proto index 56166456f23f..59a641df56e3 100644 --- a/api/envoy/config/bootstrap/v3/bootstrap.proto +++ b/api/envoy/config/bootstrap/v3/bootstrap.proto @@ -40,7 +40,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // ` for more detail. // Bootstrap :ref:`configuration overview `. -// [#next-free-field: 26] +// [#next-free-field: 27] message Bootstrap { option (udpa.annotations.versioning).previous_message_type = "envoy.config.bootstrap.v2.Bootstrap"; @@ -108,6 +108,29 @@ message Bootstrap { // identification purposes (e.g. in generated headers). core.v3.Node node = 1; + // A list of :ref:`Node ` field names + // that will be included in the context parameters of the effective + // *UdpaResourceLocator* that is sent in a discovery request when resource + // locators are used for LDS/CDS. Any non-string field will have its JSON + // encoding set as the context parameter value, with the exception of + // metadata, which will be flattened (see example below). + // + // The node context parameters act as a base layer dictionary for the context + // parameters (i.e. more specific resource specific context parameters will + // override). Field names will be prefixed with “udpa.node.” when included in + // context parameters. + // + // For example, if node_context_params is ``["user_agent_name", "metadata"]``, + // the implied context parameters might be:: + // + // node.user_agent_name: "envoy" + // node.metadata.foo: "{\"bar\": \"baz\"}" + // node.metadata.some: "42" + // node.metadata.thing: "\"thing\"" + // + // [#not-implemented-hide:] + repeated string node_context_params = 26; + // Statically specified resources. StaticResources static_resources = 2; diff --git a/api/envoy/config/bootstrap/v4alpha/bootstrap.proto b/api/envoy/config/bootstrap/v4alpha/bootstrap.proto index 24faad401e7d..d71539021e1d 100644 --- a/api/envoy/config/bootstrap/v4alpha/bootstrap.proto +++ b/api/envoy/config/bootstrap/v4alpha/bootstrap.proto @@ -38,7 +38,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // ` for more detail. // Bootstrap :ref:`configuration overview `. -// [#next-free-field: 26] +// [#next-free-field: 27] message Bootstrap { option (udpa.annotations.versioning).previous_message_type = "envoy.config.bootstrap.v3.Bootstrap"; @@ -106,6 +106,29 @@ message Bootstrap { // identification purposes (e.g. in generated headers). core.v4alpha.Node node = 1; + // A list of :ref:`Node ` field names + // that will be included in the context parameters of the effective + // *UdpaResourceLocator* that is sent in a discovery request when resource + // locators are used for LDS/CDS. Any non-string field will have its JSON + // encoding set as the context parameter value, with the exception of + // metadata, which will be flattened (see example below). + // + // The node context parameters act as a base layer dictionary for the context + // parameters (i.e. more specific resource specific context parameters will + // override). Field names will be prefixed with “udpa.node.” when included in + // context parameters. + // + // For example, if node_context_params is ``["user_agent_name", "metadata"]``, + // the implied context parameters might be:: + // + // node.user_agent_name: "envoy" + // node.metadata.foo: "{\"bar\": \"baz\"}" + // node.metadata.some: "42" + // node.metadata.thing: "\"thing\"" + // + // [#not-implemented-hide:] + repeated string node_context_params = 26; + // Statically specified resources. StaticResources static_resources = 2; diff --git a/generated_api_shadow/envoy/config/bootstrap/v3/bootstrap.proto b/generated_api_shadow/envoy/config/bootstrap/v3/bootstrap.proto index eadc77a8828a..1a8de6c141f1 100644 --- a/generated_api_shadow/envoy/config/bootstrap/v3/bootstrap.proto +++ b/generated_api_shadow/envoy/config/bootstrap/v3/bootstrap.proto @@ -40,7 +40,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // ` for more detail. // Bootstrap :ref:`configuration overview `. -// [#next-free-field: 26] +// [#next-free-field: 27] message Bootstrap { option (udpa.annotations.versioning).previous_message_type = "envoy.config.bootstrap.v2.Bootstrap"; @@ -106,6 +106,29 @@ message Bootstrap { // identification purposes (e.g. in generated headers). core.v3.Node node = 1; + // A list of :ref:`Node ` field names + // that will be included in the context parameters of the effective + // *UdpaResourceLocator* that is sent in a discovery request when resource + // locators are used for LDS/CDS. Any non-string field will have its JSON + // encoding set as the context parameter value, with the exception of + // metadata, which will be flattened (see example below). + // + // The node context parameters act as a base layer dictionary for the context + // parameters (i.e. more specific resource specific context parameters will + // override). Field names will be prefixed with “udpa.node.” when included in + // context parameters. + // + // For example, if node_context_params is ``["user_agent_name", "metadata"]``, + // the implied context parameters might be:: + // + // node.user_agent_name: "envoy" + // node.metadata.foo: "{\"bar\": \"baz\"}" + // node.metadata.some: "42" + // node.metadata.thing: "\"thing\"" + // + // [#not-implemented-hide:] + repeated string node_context_params = 26; + // Statically specified resources. StaticResources static_resources = 2; diff --git a/generated_api_shadow/envoy/config/bootstrap/v4alpha/bootstrap.proto b/generated_api_shadow/envoy/config/bootstrap/v4alpha/bootstrap.proto index e798d6195f3f..49548d4b3509 100644 --- a/generated_api_shadow/envoy/config/bootstrap/v4alpha/bootstrap.proto +++ b/generated_api_shadow/envoy/config/bootstrap/v4alpha/bootstrap.proto @@ -39,7 +39,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // ` for more detail. // Bootstrap :ref:`configuration overview `. -// [#next-free-field: 26] +// [#next-free-field: 27] message Bootstrap { option (udpa.annotations.versioning).previous_message_type = "envoy.config.bootstrap.v3.Bootstrap"; @@ -107,6 +107,29 @@ message Bootstrap { // identification purposes (e.g. in generated headers). core.v4alpha.Node node = 1; + // A list of :ref:`Node ` field names + // that will be included in the context parameters of the effective + // *UdpaResourceLocator* that is sent in a discovery request when resource + // locators are used for LDS/CDS. Any non-string field will have its JSON + // encoding set as the context parameter value, with the exception of + // metadata, which will be flattened (see example below). + // + // The node context parameters act as a base layer dictionary for the context + // parameters (i.e. more specific resource specific context parameters will + // override). Field names will be prefixed with “udpa.node.” when included in + // context parameters. + // + // For example, if node_context_params is ``["user_agent_name", "metadata"]``, + // the implied context parameters might be:: + // + // node.user_agent_name: "envoy" + // node.metadata.foo: "{\"bar\": \"baz\"}" + // node.metadata.some: "42" + // node.metadata.thing: "\"thing\"" + // + // [#not-implemented-hide:] + repeated string node_context_params = 26; + // Statically specified resources. StaticResources static_resources = 2; diff --git a/source/common/config/BUILD b/source/common/config/BUILD index 55b8cce10d0c..9b51dbb38b03 100644 --- a/source/common/config/BUILD +++ b/source/common/config/BUILD @@ -325,6 +325,18 @@ envoy_cc_library( ], ) +envoy_cc_library( + name = "udpa_context_params_lib", + srcs = ["udpa_context_params.cc"], + hdrs = ["udpa_context_params.h"], + deps = [ + "//source/common/common:macros", + "//source/common/protobuf:utility_lib", + "@com_github_cncf_udpa//udpa/core/v1:pkg_cc_proto", + "@envoy_api//envoy/config/core/v3:pkg_cc_proto", + ], +) + envoy_cc_library( name = "udpa_resource_lib", srcs = ["udpa_resource.cc"], diff --git a/source/common/config/udpa_context_params.cc b/source/common/config/udpa_context_params.cc new file mode 100644 index 000000000000..5644fb1a683a --- /dev/null +++ b/source/common/config/udpa_context_params.cc @@ -0,0 +1,91 @@ +#include "common/config/udpa_context_params.h" + +#include "common/common/macros.h" +#include "common/protobuf/utility.h" + +namespace Envoy { +namespace Config { + +namespace { + +using RenderContextParamCb = std::function; +using NodeContextRenderers = absl::flat_hash_map; + +RenderContextParamCb directStringFieldRenderer(const std::string& field) { + return [field](const envoy::config::core::v3::Node& node) -> std::string { + return MessageUtil::getStringField(node, field); + }; +} + +RenderContextParamCb localityStringFieldRenderer(const std::string& field) { + return [field](const envoy::config::core::v3::Node& node) -> std::string { + return MessageUtil::getStringField(node.locality(), field); + }; +} + +std::string buildSemanticVersionRenderer(const envoy::config::core::v3::Node& node) { + const auto& semver = node.user_agent_build_version().version(); + return fmt::format("{}.{}.{}", semver.major_number(), semver.minor_number(), semver.patch()); +} + +const NodeContextRenderers& nodeParamCbs() { + CONSTRUCT_ON_FIRST_USE(NodeContextRenderers, {"id", directStringFieldRenderer("id")}, + {"cluster", directStringFieldRenderer("cluster")}, + {"user_agent_name", directStringFieldRenderer("user_agent_name")}, + {"user_agent_version", directStringFieldRenderer("user_agent_version")}, + {"locality.region", localityStringFieldRenderer("region")}, + {"locality.zone", localityStringFieldRenderer("zone")}, + {"locality.sub_zone", localityStringFieldRenderer("sub_zone")}, + {"user_agent_build_version.version", buildSemanticVersionRenderer}); +} + +void mergeMetadataJson(Protobuf::Map& params, + const ProtobufWkt::Struct& metadata, const std::string& prefix) { + for (const auto it : metadata.fields()) { + params[prefix + it.first] = MessageUtil::getJsonStringFromMessage(it.second); + } +} + +} // namespace + +udpa::core::v1::ContextParams UdpaContextParams::encode( + const envoy::config::core::v3::Node& node, const std::vector& node_context_params, + const udpa::core::v1::ContextParams& resource_context_params, + const std::vector& client_features, + const absl::flat_hash_map& extra_resource_params) { + udpa::core::v1::ContextParams context_params; + auto& mutable_params = *context_params.mutable_params(); + // 1. Establish base layer of per-node context parameters. + for (const std::string& ncp : node_context_params) { + // First attempt field accessors known ahead of time, if that fails we consider the cases of + // metadata, either directly in the Node message, or nested in the user_agent_build_version. + if (nodeParamCbs().count(ncp) > 0) { + mutable_params["udpa.node." + ncp] = nodeParamCbs().at(ncp)(node); + } else if (ncp == "metadata") { + mergeMetadataJson(mutable_params, node.metadata(), "udpa.node.metadata."); + } else if (ncp == "user_agent_build_version.metadata") { + mergeMetadataJson(mutable_params, node.user_agent_build_version().metadata(), + "udpa.node.user_agent_build_version.metadata."); + } + } + + // 2. Overlay with context parameters from resource name. + for (const auto it : resource_context_params.params()) { + mutable_params[it.first] = it.second; + } + + // 3. Overlay with per-resource type context parameters. + for (const std::string& cf : client_features) { + mutable_params["udpa.client_feature." + cf] = "true"; + } + + // 4. Overlay with per-resource well-known attributes. + for (const auto it : extra_resource_params) { + mutable_params["udpa.resource." + it.first] = it.second; + } + + return context_params; +} + +} // namespace Config +} // namespace Envoy diff --git a/source/common/config/udpa_context_params.h b/source/common/config/udpa_context_params.h new file mode 100644 index 000000000000..23a1853365ce --- /dev/null +++ b/source/common/config/udpa_context_params.h @@ -0,0 +1,33 @@ +#pragma once + +#include "envoy/config/core/v3/base.pb.h" + +#include "absl/container/flat_hash_map.h" +#include "udpa/core/v1/context_params.pb.h" + +namespace Envoy { +namespace Config { + +// Utilities for working with context parameters. +class UdpaContextParams { +public: + /** + * Encode context parameters by following the xDS transport precedence algorithm and applying + * parameter prefixes. + * @param node reference to the local Node information. + * @param node_context_params a list of node fields to include in context parameters. + * @param resource_context_params context parameters from resource locator. + * @param client_features client feature capabilities. + * @param extra_resource_param per-resource type well known attributes. + * @return udpa::core::v1::ContextParams encoded context parameters. + */ + static udpa::core::v1::ContextParams + encode(const envoy::config::core::v3::Node& node, + const std::vector& node_context_params, + const udpa::core::v1::ContextParams& resource_context_params, + const std::vector& client_features, + const absl::flat_hash_map& extra_resource_params); +}; + +} // namespace Config +} // namespace Envoy diff --git a/source/common/config/udpa_resource.h b/source/common/config/udpa_resource.h index 5f90dcf1b042..c94a19c54261 100644 --- a/source/common/config/udpa_resource.h +++ b/source/common/config/udpa_resource.h @@ -1,3 +1,5 @@ +#pragma once + #include "envoy/common/exception.h" #include "absl/strings/string_view.h" diff --git a/test/common/config/BUILD b/test/common/config/BUILD index ba12f30e6b46..e870e01a733a 100644 --- a/test/common/config/BUILD +++ b/test/common/config/BUILD @@ -440,15 +440,32 @@ envoy_cc_test( ], ) +envoy_cc_test( + name = "udpa_context_params_test", + srcs = ["udpa_context_params_test.cc"], + deps = [ + ":udpa_test_utility_lib", + "//source/common/config:udpa_context_params_lib", + "//test/test_common:logging_lib", + "//test/test_common:utility_lib", + ], +) + envoy_cc_test( name = "udpa_resource_test", srcs = ["udpa_resource_test.cc"], deps = [ + ":udpa_test_utility_lib", "//source/common/config:udpa_resource_lib", "//test/test_common:utility_lib", ], ) +envoy_cc_test_library( + name = "udpa_test_utility_lib", + hdrs = ["udpa_test_utility.h"], +) + envoy_proto_library( name = "version_converter_proto", srcs = ["version_converter.proto"], diff --git a/test/common/config/udpa_context_params_test.cc b/test/common/config/udpa_context_params_test.cc new file mode 100644 index 000000000000..98ea4b758053 --- /dev/null +++ b/test/common/config/udpa_context_params_test.cc @@ -0,0 +1,156 @@ +#include "common/config/udpa_context_params.h" + +#include "test/common/config/udpa_test_utility.h" +#include "test/test_common/logging.h" +#include "test/test_common/utility.h" + +#include "gtest/gtest.h" +#include "udpa_test_utility.h" + +using ::testing::Pair; + +namespace Envoy { +namespace Config { +namespace { + +// Validate all the node parameter renderers (except user_agent_build_version, which has its own +// test below). +TEST(UdpaContextParamsTest, NodeAll) { + envoy::config::core::v3::Node node; + TestUtility::loadFromYaml(R"EOF( + id: some_id + cluster: some_cluster + user_agent_name: xds_client + user_agent_version: 1.2.3 + locality: + region: some_region + zone: some_zone + sub_zone: some_sub_zone + metadata: + foo: true + bar: "a" + baz: 42 + )EOF", + node); + const auto context_params = UdpaContextParams::encode( + node, + {"id", "cluster", "user_agent_name", "user_agent_version", "locality.region", "locality.zone", + "locality.sub_zone", "metadata"}, + {}, {}, {}); + EXPECT_CONTEXT_PARAMS( + context_params, Pair("udpa.node.cluster", "some_cluster"), Pair("udpa.node.id", "some_id"), + Pair("udpa.node.locality.sub_zone", "some_sub_zone"), + Pair("udpa.node.locality.zone", "some_zone"), + Pair("udpa.node.locality.region", "some_region"), Pair("udpa.node.metadata.bar", "\"a\""), + Pair("udpa.node.metadata.baz", "42"), Pair("udpa.node.metadata.foo", "true"), + Pair("udpa.node.user_agent_name", "xds_client"), + Pair("udpa.node.user_agent_version", "1.2.3")); +} + +// Validate that we can select a subset of node parameters. +TEST(UdpaContextParamsTest, NodeParameterSelection) { + envoy::config::core::v3::Node node; + TestUtility::loadFromYaml(R"EOF( + id: some_id + cluster: some_cluster + user_agent_name: xds_client + user_agent_version: 1.2.3 + locality: + region: some_region + zone: some_zone + sub_zone: some_sub_zone + metadata: + foo: true + bar: "a" + baz: 42 + )EOF", + node); + const auto context_params = UdpaContextParams::encode( + node, {"cluster", "user_agent_version", "locality.region", "locality.sub_zone"}, {}, {}, {}); + EXPECT_CONTEXT_PARAMS(context_params, Pair("udpa.node.cluster", "some_cluster"), + Pair("udpa.node.locality.sub_zone", "some_sub_zone"), + Pair("udpa.node.locality.region", "some_region"), + Pair("udpa.node.user_agent_version", "1.2.3")); +} + +// Validate user_agent_build_version renderers. +TEST(UdpaContextParamsTest, NodeUserAgentBuildVersion) { + envoy::config::core::v3::Node node; + TestUtility::loadFromYaml(R"EOF( + user_agent_build_version: + version: + major_number: 1 + minor_number: 2 + patch: 3 + metadata: + foo: true + bar: "a" + baz: 42 + )EOF", + node); + const auto context_params = UdpaContextParams::encode( + node, {"user_agent_build_version.version", "user_agent_build_version.metadata"}, {}, {}, {}); + EXPECT_CONTEXT_PARAMS(context_params, + Pair("udpa.node.user_agent_build_version.metadata.bar", "\"a\""), + Pair("udpa.node.user_agent_build_version.metadata.baz", "42"), + Pair("udpa.node.user_agent_build_version.metadata.foo", "true"), + Pair("udpa.node.user_agent_build_version.version", "1.2.3")); +} + +// Validate that resource locator context parameters are pass-thru. +TEST(UdpaContextParamsTest, ResoureContextParams) { + udpa::core::v1::ContextParams resource_context_params; + TestUtility::loadFromYaml(R"EOF( + params: + foo: "\"some_string\"" + bar: "123" + baz: "true" + )EOF", + resource_context_params); + const auto context_params = UdpaContextParams::encode({}, {}, resource_context_params, {}, {}); + EXPECT_CONTEXT_PARAMS(context_params, Pair("bar", "123"), Pair("baz", "true"), + Pair("foo", "\"some_string\"")); +} + +// Validate client feature capabilities context parameter transform. +TEST(UdpaContextParamsTest, ClientFeatureCapabilities) { + const auto context_params = + UdpaContextParams::encode({}, {}, {}, {"some.feature", "another.feature"}, {}); + EXPECT_CONTEXT_PARAMS(context_params, Pair("udpa.client_feature.another.feature", "true"), + Pair("udpa.client_feature.some.feature", "true")); +} + +// Validate per-resource well-known attributes transform. +TEST(UdpaContextParamsTest, ResourceWktAttribs) { + const auto context_params = + UdpaContextParams::encode({}, {}, {}, {}, {{"foo", "1"}, {"bar", "2"}}); + EXPECT_CONTEXT_PARAMS(context_params, Pair("udpa.resource.foo", "1"), + Pair("udpa.resource.bar", "2")); +} + +// Validate that the precedence relationships in the specification hold. +TEST(UdpaContextParamsTest, Layering) { + envoy::config::core::v3::Node node; + TestUtility::loadFromYaml(R"EOF( + id: some_id + cluster: some_cluster + )EOF", + node); + udpa::core::v1::ContextParams resource_context_params; + TestUtility::loadFromYaml(R"EOF( + params: + id: another_id + udpa.node.cluster: another_cluster + )EOF", + resource_context_params); + const auto context_params = UdpaContextParams::encode( + node, {"id", "cluster"}, resource_context_params, {"id"}, {{"cluster", "huh"}}); + EXPECT_CONTEXT_PARAMS(context_params, Pair("id", "another_id"), + Pair("udpa.client_feature.id", "true"), + Pair("udpa.node.cluster", "another_cluster"), + Pair("udpa.node.id", "some_id"), Pair("udpa.resource.cluster", "huh")); +} + +} // namespace +} // namespace Config +} // namespace Envoy diff --git a/test/common/config/udpa_resource_test.cc b/test/common/config/udpa_resource_test.cc index 0cf6aeef45f8..b9629588c4a6 100644 --- a/test/common/config/udpa_resource_test.cc +++ b/test/common/config/udpa_resource_test.cc @@ -1,19 +1,12 @@ #include "common/config/udpa_resource.h" +#include "test/common/config/udpa_test_utility.h" #include "test/test_common/utility.h" #include "gtest/gtest.h" using ::testing::ElementsAre; using ::testing::Pair; -using ::testing::UnorderedElementsAre; - -#define EXPECT_CONTEXT_PARAMS(context_params, ...) \ - { \ - std::map param_map((context_params).begin(), \ - (context_params).end()); \ - EXPECT_THAT(param_map, UnorderedElementsAre(__VA_ARGS__)); \ - } namespace Envoy { namespace Config { @@ -64,7 +57,7 @@ TEST(UdpaResourceNameTest, DecodeSuccess) { EXPECT_EQ("f123%/?#o", resource_name.authority()); EXPECT_EQ("envoy.config.listener.v3.Listener", resource_name.resource_type()); EXPECT_THAT(resource_name.id(), ElementsAre("b%:/?#[]ar", "", "baz")); - EXPECT_CONTEXT_PARAMS(resource_name.context().params(), Pair("%#[]&=", "bar"), + EXPECT_CONTEXT_PARAMS(resource_name.context(), Pair("%#[]&=", "bar"), Pair("%#[]&=ab", "cde%#[]&=f"), Pair("foo", "%#[]&=")); } @@ -75,7 +68,7 @@ TEST(UdpaResourceLocatorTest, DecodeSuccess) { EXPECT_EQ("f123%/?#o", resource_locator.authority()); EXPECT_EQ("envoy.config.listener.v3.Listener", resource_locator.resource_type()); EXPECT_THAT(resource_locator.id(), ElementsAre("b%:/?#[]ar", "", "baz")); - EXPECT_CONTEXT_PARAMS(resource_locator.exact_context().params(), Pair("%#[]&=", "bar"), + EXPECT_CONTEXT_PARAMS(resource_locator.exact_context(), Pair("%#[]&=", "bar"), Pair("%#[]&=ab", "cde%#[]&=f"), Pair("foo", "%#[]&=")); EXPECT_EQ(2, resource_locator.directives().size()); EXPECT_EQ("some_en%#[],try", resource_locator.directives()[0].entry()); @@ -152,7 +145,7 @@ TEST(UdpaResourceLocatorTest, Schemes) { EXPECT_EQ("foo", resource_locator.authority()); EXPECT_EQ("bar", resource_locator.resource_type()); EXPECT_THAT(resource_locator.id(), ElementsAre("baz", "blah")); - EXPECT_CONTEXT_PARAMS(resource_locator.exact_context().params(), Pair("a", "b")); + EXPECT_CONTEXT_PARAMS(resource_locator.exact_context(), Pair("a", "b")); EXPECT_EQ(1, resource_locator.directives().size()); EXPECT_EQ("m", resource_locator.directives()[0].entry()); EXPECT_EQ("udpa://foo/bar/baz/blah?a=b#entry=m", @@ -165,7 +158,7 @@ TEST(UdpaResourceLocatorTest, Schemes) { EXPECT_EQ("foo", resource_locator.authority()); EXPECT_EQ("bar", resource_locator.resource_type()); EXPECT_THAT(resource_locator.id(), ElementsAre("baz", "blah")); - EXPECT_CONTEXT_PARAMS(resource_locator.exact_context().params(), Pair("a", "b")); + EXPECT_CONTEXT_PARAMS(resource_locator.exact_context(), Pair("a", "b")); EXPECT_EQ(1, resource_locator.directives().size()); EXPECT_EQ("m", resource_locator.directives()[0].entry()); EXPECT_EQ("http://foo/bar/baz/blah?a=b#entry=m", diff --git a/test/common/config/udpa_test_utility.h b/test/common/config/udpa_test_utility.h new file mode 100644 index 000000000000..10e3e66f5fbb --- /dev/null +++ b/test/common/config/udpa_test_utility.h @@ -0,0 +1,16 @@ +#pragma once + +#include "gtest/gtest.h" + +#define EXPECT_CONTEXT_PARAMS(context_params, ...) \ + { \ + std::map param_map((context_params).params().begin(), \ + (context_params).params().end()); \ + EXPECT_THAT(param_map, ::testing::UnorderedElementsAre(__VA_ARGS__)); \ + } + +namespace Envoy { +namespace Config { +namespace {} // namespace +} // namespace Config +} // namespace Envoy From 276d9c3de21121f5c0245443e9109a8441a35536 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Tue, 1 Sep 2020 19:02:57 -0400 Subject: [PATCH 2/4] Add 'renderers' to spelling dictionary. Signed-off-by: Harvey Tuch --- tools/spelling/spelling_dictionary.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index 4647007122f2..d2bf372e07cb 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -962,6 +962,7 @@ rele releasor reloadable remoting +renderers reparse repeatability reperform From 7811b03620874763ce90a06e5c74ae24b38818b4 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Tue, 8 Sep 2020 18:00:22 -0400 Subject: [PATCH 3/4] Release build fixes. Signed-off-by: Harvey Tuch --- source/common/config/udpa_context_params.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source/common/config/udpa_context_params.cc b/source/common/config/udpa_context_params.cc index 5644fb1a683a..4a12246e8245 100644 --- a/source/common/config/udpa_context_params.cc +++ b/source/common/config/udpa_context_params.cc @@ -41,7 +41,7 @@ const NodeContextRenderers& nodeParamCbs() { void mergeMetadataJson(Protobuf::Map& params, const ProtobufWkt::Struct& metadata, const std::string& prefix) { - for (const auto it : metadata.fields()) { + for (const auto& it : metadata.fields()) { params[prefix + it.first] = MessageUtil::getJsonStringFromMessage(it.second); } } @@ -70,7 +70,7 @@ udpa::core::v1::ContextParams UdpaContextParams::encode( } // 2. Overlay with context parameters from resource name. - for (const auto it : resource_context_params.params()) { + for (const auto& it : resource_context_params.params()) { mutable_params[it.first] = it.second; } @@ -80,7 +80,7 @@ udpa::core::v1::ContextParams UdpaContextParams::encode( } // 4. Overlay with per-resource well-known attributes. - for (const auto it : extra_resource_params) { + for (const auto& it : extra_resource_params) { mutable_params["udpa.resource." + it.first] = it.second; } From dfd353c6852b8827b69fb42d54fe4cef80bc972a Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Wed, 9 Sep 2020 16:51:00 -0400 Subject: [PATCH 4/4] Explicit node field names. Signed-off-by: Harvey Tuch --- api/envoy/config/bootstrap/v3/bootstrap.proto | 13 ++++++++++++- api/envoy/config/bootstrap/v4alpha/bootstrap.proto | 13 ++++++++++++- .../envoy/config/bootstrap/v3/bootstrap.proto | 13 ++++++++++++- .../envoy/config/bootstrap/v4alpha/bootstrap.proto | 13 ++++++++++++- 4 files changed, 48 insertions(+), 4 deletions(-) diff --git a/api/envoy/config/bootstrap/v3/bootstrap.proto b/api/envoy/config/bootstrap/v3/bootstrap.proto index 59a641df56e3..a1e981fcbdda 100644 --- a/api/envoy/config/bootstrap/v3/bootstrap.proto +++ b/api/envoy/config/bootstrap/v3/bootstrap.proto @@ -113,7 +113,18 @@ message Bootstrap { // *UdpaResourceLocator* that is sent in a discovery request when resource // locators are used for LDS/CDS. Any non-string field will have its JSON // encoding set as the context parameter value, with the exception of - // metadata, which will be flattened (see example below). + // metadata, which will be flattened (see example below). The supported field + // names are: + // - "cluster" + // - "id" + // - "locality.region" + // - "locality.sub_zone" + // - "locality.zone" + // - "metadata" + // - "user_agent_build_version.metadata" + // - "user_agent_build_version.version" + // - "user_agent_name" + // - "user_agent_version" // // The node context parameters act as a base layer dictionary for the context // parameters (i.e. more specific resource specific context parameters will diff --git a/api/envoy/config/bootstrap/v4alpha/bootstrap.proto b/api/envoy/config/bootstrap/v4alpha/bootstrap.proto index d71539021e1d..989ecd30ddc4 100644 --- a/api/envoy/config/bootstrap/v4alpha/bootstrap.proto +++ b/api/envoy/config/bootstrap/v4alpha/bootstrap.proto @@ -111,7 +111,18 @@ message Bootstrap { // *UdpaResourceLocator* that is sent in a discovery request when resource // locators are used for LDS/CDS. Any non-string field will have its JSON // encoding set as the context parameter value, with the exception of - // metadata, which will be flattened (see example below). + // metadata, which will be flattened (see example below). The supported field + // names are: + // - "cluster" + // - "id" + // - "locality.region" + // - "locality.sub_zone" + // - "locality.zone" + // - "metadata" + // - "user_agent_build_version.metadata" + // - "user_agent_build_version.version" + // - "user_agent_name" + // - "user_agent_version" // // The node context parameters act as a base layer dictionary for the context // parameters (i.e. more specific resource specific context parameters will diff --git a/generated_api_shadow/envoy/config/bootstrap/v3/bootstrap.proto b/generated_api_shadow/envoy/config/bootstrap/v3/bootstrap.proto index 1a8de6c141f1..27c69eae991b 100644 --- a/generated_api_shadow/envoy/config/bootstrap/v3/bootstrap.proto +++ b/generated_api_shadow/envoy/config/bootstrap/v3/bootstrap.proto @@ -111,7 +111,18 @@ message Bootstrap { // *UdpaResourceLocator* that is sent in a discovery request when resource // locators are used for LDS/CDS. Any non-string field will have its JSON // encoding set as the context parameter value, with the exception of - // metadata, which will be flattened (see example below). + // metadata, which will be flattened (see example below). The supported field + // names are: + // - "cluster" + // - "id" + // - "locality.region" + // - "locality.sub_zone" + // - "locality.zone" + // - "metadata" + // - "user_agent_build_version.metadata" + // - "user_agent_build_version.version" + // - "user_agent_name" + // - "user_agent_version" // // The node context parameters act as a base layer dictionary for the context // parameters (i.e. more specific resource specific context parameters will diff --git a/generated_api_shadow/envoy/config/bootstrap/v4alpha/bootstrap.proto b/generated_api_shadow/envoy/config/bootstrap/v4alpha/bootstrap.proto index 49548d4b3509..bf70a52e9d1d 100644 --- a/generated_api_shadow/envoy/config/bootstrap/v4alpha/bootstrap.proto +++ b/generated_api_shadow/envoy/config/bootstrap/v4alpha/bootstrap.proto @@ -112,7 +112,18 @@ message Bootstrap { // *UdpaResourceLocator* that is sent in a discovery request when resource // locators are used for LDS/CDS. Any non-string field will have its JSON // encoding set as the context parameter value, with the exception of - // metadata, which will be flattened (see example below). + // metadata, which will be flattened (see example below). The supported field + // names are: + // - "cluster" + // - "id" + // - "locality.region" + // - "locality.sub_zone" + // - "locality.zone" + // - "metadata" + // - "user_agent_build_version.metadata" + // - "user_agent_build_version.version" + // - "user_agent_name" + // - "user_agent_version" // // The node context parameters act as a base layer dictionary for the context // parameters (i.e. more specific resource specific context parameters will