xDS resource patching

[kyessenov]

As a continuation of the incremental xDS theme, it would be good to provide a general ability for xDS servers to patch individual xDS resources. We have two use cases in mind:

• large endpoint lists in cluster load assignments;
• break-glass modifications applied to generic xDS responses;

The ability would require implementing an xDS-aware patch semantics in Envoy. This may need support from additional proto annotations to provide customizations to how lists and maps are combined (see https://github.com/kubernetes/community/blob/master/contributors/devel/sig-api-machinery/strategic-merge-patch.md, for example).

/cc @howardjohn @mandarjog

[htuch]

We have had similar discussions in the context of UDPA FWIW. The main objection is the client complexity; other clients such as gRPC, which might not share the use cases that Envoy has, should not have to take on the implementation complexity. If we can figure out a lightweight approach (lighterweight than the linked strategic merge patch, things could be more palatable.

[kyessenov]

Let me sketch out a minimalistic proposal here, and let me know if that looks like the right direction.

1. A patch is a tuple (extended field path, protobuf message, operation). Patches are applied in a sequence. Patches that don't match extended field paths are no-ops.

2. Extended field path is a sequence of one or more selectors starting from resources field in the response. The target of the field path is an embedded message field value (singular or one of repeated). The field path may not match, in which case the patch is a no-op. The field paths are resolved dynamically to handle the polymorphic protobuf.Any. Each selector is one of:
   2a. f refers to a singular message field f in the parent message.
   2b. f[i] refers to i-th value of a repeated message field f in the parent message. This supports negative indexes, where -1 refers to the last element.
   2c. f[g = string_literal] refers to a value in the repeated message field f for which field g has the specified literal value. As a special case, f[string_literal] refers to a map entry value for a map field f with the given key.

3. The protobuf message must be of the same message type as the value of the field path for a patch to apply.

4. There are four supported operations:
   4a. Merge: Update the value of the field path with the given message by applying the standard protobuf merge operation: rewrite primitive values, recursively merge message values, and concatenate repeated values.
   4b. Delete: Delete the referenced field value (clear it for an embedded message field, delete from repeated elements, or remove from a map). The provided message is ignored.
   4c. Prepend: Only applicable if the field path value refers to a value in a repeated field. Insert the given value right before the referenced value.
   4d. Append: Only applicable if the field path value refers to a value in a repeated field. Insert the given value right after the referenced value.

Examples:

1. LDS patch to update HTTP buffer filter config: (resources[name=inbound].filter_chains[0].filters[name=HCM].typed_config.http_filters[name=envoy.filters.http.buffer], {max_request_bytes: 1024}, Merge)
2. RDS patch to delete a wildcard virtual host:
   (resources[name=inbound].virtual_hosts[name=wildcard], {}, Delete)
3. CDS patch to change http2 settings:
   (resources[name=xds], {http2_protocol_options: {}}, Merge)
4. LDS patch to prepend a special HTTP filter in the filter chain:
   (resources[name=inbound].filter_chains[0].filters[name=HCM].typed_config.http_filters, {name="my_filter", typed_config = "..."}, Prepend)

[mattklein123]

@kyessenov I would suggest that we have a short meeting to discuss this before we dive too far into the implementation details. As @htuch mentioned, we have actually already discussed the concept of merging quite a bit in the context of federation. I have gone back and forth several times on whether we should implement something like this or not (with my current thinking erring on the side of yes TBH), but I think we should discuss motivations first to make sure we are doing this for the right reasons.

[kyessenov]

SGTM, would be happy to chat more. CC @rshriram who worked on xDS patching as an Istio API.

[rshriram]

What is the rationale for pushing the patching mechanism to the data plane? There was a non trivial amount of struct/any business that we had to handle in the control plane in order to do the protobuf merging. This is still not complete but at the very least, we have more flexibility to do this in the control plane than in the data plane IMO. On top of this, we also have to handle validation issues (invalid listeners, etc.).

Pushing the patching mechanism to the data plane is simply moving all these problems from Pilot to Envoy. In a scenario where we manage to cache these patched XDS resources, we have the benefit of doing the patching once per N proxies. We would lose that benefit as well if we do the patching in the data plane.

[kyessenov]

@rshriram Specifically for istio, we are trying to solve two problems:

• O(n^2) config size on the wire when changes are propagated linearly; for example, EDS payload being sent with 1,2,3, and up to n endpoints. This calls for an incremental EDS payload.
• Istio extensions applicable across control planes, e.g. I'd guess similar to federation. We would like to have a declarative description of where to inject Istio extensions that can be applied across multiple control planes (e.g. various versions of Istio Pilot). This is not a full replacement for EnvoyFilter since for patching, we don't assume any prior knowledge about the semantics of names or config layout, which means Istio would be generating xDS patches.

[htuch]

Please include me in any meeting you folks setup, there are implications for UDPA that I'd like to get clear. I'd prefer that we make this technique as dead simple to implement as possible; if we have to write more than ~100 lines-of-code to do the merge, I worry that non-Envoy xDS/UDPA clients are going to have a tough time implementing the feature. CC @markdroth

[markdroth]

I'd like to be included in this discussion too. I'm really concerned about the complexity and manageability of protobuf merging.

I do agree that we need a form of EDS that is incremental at the granularity of individual endpoints, and I've mentioned this to Harvey before. But I think that use-case is something we can address more specifically via a separate first-class mechanism (e.g., maybe making each endpoint its own resource) rather than via a patching mechanism.

[howardjohn]

+1 for being able to send individual endpoints. Is this any different than delta xDS though?

I agree with @rshriram though for the most part, it seems this is better suited for the control plane to handle

[kyessenov]

I agree that simplicity is paramount. I think the real problem is that the config "modules" that Envoy provides (which is basically an individual xDS resource) does not line up well with the config intent. For example:

• an HTTP filter that needs metadata from virtual hosts and individual routes. Due to how config is spread out, that requires modifying LDS listener and RDS route. RDS route modification is a fairly structured patch.

It'd be great if we can change the "module" boundaries (e.g. subdivide endpoints object), but I'm concerned this is going to be an on-going problem for other use cases.

[lambdai]

@howardjohn I think delta and incremental granular is per X in XDS where X has a name. So adding single cluster/listener/route is possible
For Eds the named resource is cluster, so we cannot yet add/delete single endpoint

[mattklein123]

cc @wgallagher @gkleiman

[htuch]

If we do want to specify overlay paths, one option we might want to think about is https://goessner.net/articles/JsonPath/. Basically, being able to describe a path from root to a subtree in a message. Proto3/JSON are canonically mapped, so it seems somewhat reusable.