config: fix delta xDS's use of (un)subscribe fields, more explicit protocol spec

api/XDS_PROTOCOL.md

@@ -348,7 +348,7 @@ client spontaneously requests the "wc" resource.
 ![Incremental session example](diagrams/incremental.svg)
 
 On reconnect the Incremental xDS client may tell the server of its known
-resources to avoid resending them over the network.
+resources to avoid resending them over the network. Because no state is assumed to be preserved from the previous stream, the reconnecting client must provide the server with all resource names it is interested in.
 
 ![Incremental reconnect example](diagrams/incremental-reconnect.svg)
 
@@ -358,17 +358,24 @@ identified by the alias field in the resource of a `DeltaDiscoveryResponse`. The
 be returned in the name field in the resource of a `DeltaDiscoveryResponse`.
 
 #### Subscribing to Resources
-Envoy can send either an alias or the name of a resource in the `resource_names_subscribe` field of
-a `DeltaDiscoveryRequest` in order to subscribe to a resource. Envoy should check both the names and
-aliases of resources in order to determine whether the entity in question has been subscribed to.
+The client can send either an alias or the name of a resource in the `resource_names_subscribe`
+field of a `DeltaDiscoveryRequest` in order to subscribe to a resource. Both the names and aliases
+of resources should be checked in order to determine whether the entity in question has been
+subscribed to.
+
+A `resource_names_subscribe` field may contain resource names that the server believes the client
+is already subscribed to, and furthermore has the most recent versions of. However, the server
+*must* still provide those resources in the response; due to implementation details hidden from
+the server, the client may have "forgotten" those resources despite apparently remaining subscribed.
 
 #### Unsubscribing from Resources
-Envoy will keep track of a per resource reference count internally. This count will keep track of the
-total number of aliases/resource names that are currently subscribed to. When the reference count
-reaches zero, Envoy will send a `DeltaDiscoveryRequest` containing the resource name of the resource
-to unsubscribe from in the `resource_names_unsubscribe` field. When Envoy unsubscribes from a resource,
-it should check for both the resource name and all aliases and appropriately update all resources
-that reference either.
+When a client loses interest in some resources, it will indicate that with the
+`resource_names_unsubscribe` field of a `DeltaDiscoveryRequest`. As with `resource_names_subscribe`,
+these may be resource names or aliases.
+
+A `resource_names_unsubscribe` field may contain superfluous resource names, which the server
+thought the client was already not subscribed to. The server must cleanly process such a request;
+it can simply ignore these phantom unsubscriptions.
 
 ## REST-JSON polling subscriptions
 

include/envoy/config/grpc_mux.h

@@ -82,15 +82,15 @@ class GrpcMux {
    * Start a configuration subscription asynchronously for some API type and resources.
    * @param type_url type URL corresponding to xDS API, e.g.
    * type.googleapis.com/envoy.api.v2.Cluster.
-   * @param resources vector of resource names to watch for. If this is empty, then all
+   * @param resources set of resource names to watch for. If this is empty, then all
    *                  resources for type_url will result in callbacks.
    * @param callbacks the callbacks to be notified of configuration updates. These must be valid
    *                  until GrpcMuxWatch is destroyed.
    * @return GrpcMuxWatchPtr a handle to cancel the subscription with. E.g. when a cluster goes
    * away, its EDS updates should be cancelled by destroying the GrpcMuxWatchPtr.
    */
   virtual GrpcMuxWatchPtr subscribe(const std::string& type_url,
-                                    const std::vector<std::string>& resources,
+                                    const std::set<std::string>& resources,
                                     GrpcMuxCallbacks& callbacks) PURE;
 
   /**

include/envoy/config/subscription.h

@@ -69,18 +69,18 @@ class Subscription {
   /**
    * Start a configuration subscription asynchronously. This should be called once and will continue
    * to fetch throughout the lifetime of the Subscription object.
-   * @param resources vector of resource names to fetch.
+   * @param resources set of resource names to fetch.
    * @param callbacks the callbacks to be notified of configuration updates. The callback must not
    *        result in the deletion of the Subscription object.
    */
-  virtual void start(const std::vector<std::string>& resources,
-                     SubscriptionCallbacks& callbacks) PURE;
+  virtual void start(const std::set<std::string>& resources, SubscriptionCallbacks& callbacks) PURE;
 
   /**
    * Update the resources to fetch.
-   * @param resources vector of resource names to fetch.
+   * @param resources vector of resource names to fetch. It's a (not unordered_)set so that it can
+   * be passed to std::set_difference, which must be given sorted collections.
    */
-  virtual void updateResources(const std::vector<std::string>& resources) PURE;
+  virtual void updateResources(const std::set<std::string>& update_to_these_names) PURE;
 };
 
 /**

source/common/common/utility.cc

@@ -375,6 +375,15 @@ std::string StringUtil::join(const std::vector<std::string>& source, const std::
   return ret.substr(0, ret.length() - delimiter.length());
 }
 
+std::string StringUtil::join(const std::set<std::string>& source, const std::string& delimiter) {
+  std::ostringstream buf;
+  std::copy(source.begin(), source.end(),
+            std::ostream_iterator<std::string>(buf, delimiter.c_str()));
+  std::string ret = buf.str();
+  // copy will always end with an extra delimiter, we remove it here.
+  return ret.substr(0, ret.length() - delimiter.length());
+}
+
 std::string StringUtil::subspan(absl::string_view source, size_t start, size_t end) {
   return std::string(source.data() + start, end - start);
 }

source/common/common/utility.h

@@ -297,6 +297,14 @@ class StringUtil {
    */
   static std::string join(const std::vector<std::string>& source, const std::string& delimiter);
 
+  /**
+   * Join elements of a sorted set into a string delimited by delimiter.
+   * @param source supplies the strings to join.
+   * @param delimiter supplies the delimiter to join them together.
+   * @return string combining elements of `source` with `delimiter` in between each element.
+   */
+  static std::string join(const std::set<std::string>& source, const std::string& delimiter);
+
   /**
    * Version of substr() that operates on a start and end index instead of a start index and a
    * length.