Review feedback.

Signed-off-by: Harvey Tuch <[email protected]>
htuch · Jun 23, 2020 · 7ea449b · 7ea449b
1 parent 40b252c
commit 7ea449b
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 9 deletions.
diff --git a/api/bazel/repository_locations.bzl b/api/bazel/repository_locations.bzl
@@ -13,10 +13,11 @@ GOOGLEAPIS_SHA = "a45019af4d3290f02eaeb1ce10990166978c807cb33a9692141a076ba46d14
 PROMETHEUS_GIT_SHA = "99fa1f4be8e564e8a6b613da7fa6f46c9edafc6c"  # Nov 17, 2017
 PROMETHEUS_SHA = "783bdaf8ee0464b35ec0c8704871e1e72afa0005c3f3587f65d9d6694bf3911b"
 
-UDPA_GIT_SHA = "54ff491c3169db0ee73efc6736cf98039105b42d"  # June 21, 2020
+UDPA_GIT_SHA = "07b17b004e9aadb53ff9bb904fca432c1e6c2f9d"  # June 21, 2020
+
 # TODO(htuch): DO NOT COMMIT until https://github.com/cncf/udpa/pull/29 merges and this
 # change is removed.
-UDPA_SHA256 = "83caf3d24918d380f44d90271f1795dc1a07c77832619e062ed30342abf45584"
+UDPA_SHA256 = "0be50f8c9353c96c570339bcfc36ab26b100e8caa1d0eb5385869a8aa89071c7"
 
 ZIPKINAPI_RELEASE = "0.2.2"  # Aug 23, 2019
 ZIPKINAPI_SHA256 = "688c4fe170821dd589f36ec45aaadc03a618a40283bc1f97da8fa11686fc816b"

diff --git a/generated_api_shadow/bazel/repository_locations.bzl b/generated_api_shadow/bazel/repository_locations.bzl
diff --git a/source/common/config/udpa_resource.cc b/source/common/config/udpa_resource.cc
@@ -38,13 +38,13 @@ std::string UdpaResourceName::encodeUri(const udpa::core::v1::ResourceName& reso
   const std::string query_params =
       query_param_components.empty() ? "" : "?" + absl::StrJoin(query_param_components, "&");
   return absl::StrCat("udpa://", authority, "/", resource_name.qualified_type(),
-                      path.empty() && query_params.empty() ? "" : "/", path, query_params);
+                      path.empty() ? "" : "/", path, query_params);
 }
 
 udpa::core::v1::ResourceName UdpaResourceName::decodeUri(absl::string_view resource_uri) {
-  if (!absl::StartsWith(resource_uri, "udpa://")) {
+  if (!absl::StartsWith(resource_uri, "udpa:")) {
     throw UdpaResourceName::DecodeException(
-        fmt::format("{} does not have udpa:// scheme", resource_uri));
+        fmt::format("{} does not have an udpa scheme", resource_uri));
   }
   absl::string_view host, path;
   Http::Utility::extractHostPathFromUri(resource_uri, host, path);

diff --git a/test/common/config/udpa_resource_test.cc b/test/common/config/udpa_resource_test.cc
@@ -19,10 +19,12 @@ const std::string EscapedUriWithManyQueryParams =
 // TODO(htuch): write a fuzzer that validates this property as well.
 TEST(UdpaResourceNameTest, DecodeEncode) {
   const std::vector<std::string> uris = {
+      "udpa:///envoy.config.listener.v3.Listener",
       "udpa://foo/envoy.config.listener.v3.Listener",
       "udpa://foo/envoy.config.listener.v3.Listener/bar",
       "udpa://foo/envoy.config.listener.v3.Listener/bar/baz",
-      "udpa://foo/envoy.config.listener.v3.Listener/?ab=cde",
+      "udpa://foo/envoy.config.listener.v3.Listener/bar////baz",
+      "udpa://foo/envoy.config.listener.v3.Listener?ab=cde",
       "udpa://foo/envoy.config.listener.v3.Listener/bar?ab=cd",
       "udpa://foo/envoy.config.listener.v3.Listener/bar/baz?ab=cde",
       "udpa://foo/envoy.config.listener.v3.Listener/bar/baz?ab=",
@@ -53,12 +55,22 @@ TEST(UdpaResourceNameTest, DecodeSuccess) {
   EXPECT_EQ("%#&=", resource_name.context().params().at("foo"));
 }
 
+// Validate that the URI decoding behaves with a near-empty UDPA resource name.
+TEST(UdpaResourceNameTest, DecodeEmpty) {
+  const auto resource_name =
+      UdpaResourceName::decodeUri("udpa:///envoy.config.listener.v3.Listener");
+  EXPECT_TRUE(resource_name.authority().empty());
+  EXPECT_EQ("envoy.config.listener.v3.Listener", resource_name.qualified_type());
+  EXPECT_TRUE(resource_name.id().empty());
+  EXPECT_TRUE(resource_name.context().params().empty());
+}
+
 // Negative tests for URI decoding.
 TEST(UdpaResourceNameTest, DecodeFail) {
   {
     EXPECT_THROW_WITH_MESSAGE(UdpaResourceName::decodeUri("foo://"),
                               UdpaResourceName::DecodeException,
-                              "foo:// does not have udpa:// scheme");
+                              "foo:// does not have an udpa scheme");
   }
   {
     EXPECT_THROW_WITH_MESSAGE(UdpaResourceName::decodeUri("udpa://foo"),