Allow request_headers_to_add in weighted clusters

[mandarjog]

Allow ability to configure %headers_to_% at weighted cluster level

Description:
This gives a way to inject headers that are specific to route+weighted cluster combination.

The specific use case I am looking at is to forward x-destination-cluster header and some related information to the actual upstream cluster. The upstream envoy can then use cluster_header: x-destination-cluster in its vhost configuration.

This will be used by Istio to inject intermediaries between two workloads.
With subset_lb there would be additional headers sent.

Following is an example where the proposed istio via directive is used.

destination:
    name: httpbin
  precedence: 1
  route:
 - labels:
      version: v50
    weight: 90
  - labels:
      version: v51
    weight: 10
    Via:
      Service: inter
      Labels:
        Version: v1

This rule says that traffic to httpbin host should be sent via the inter service.

The original rule

    virtual_hosts:
    - domains:
      - httpbin
      - 10.7.250.118:8000
      - 10.7.250.118
      name: httpbin.default.svc.cluster.local|http
      routes:
      - decorator:
          operation: details-default
        match:
          prefix: /
        route:
          timeout: 0s
          weighted_clusters:
            clusters:
            - name: out.httpbin.default.svc.cluster.local|http|version=v50
              weight: 90
            - name: out.httpbin.default.svc.cluster.local|http|version=v51
              weight: 10

becomes --

    virtual_hosts:
    - domains:
      - httpbin
      - 10.7.250.118:8000
      - 10.7.250.118
      name: httpbin.default.svc.cluster.local|http
      routes:
      - decorator:
          operation: details-default
        match:
          prefix: /
        route:
          timeout: 0s
          weighted_clusters:
            clusters:
            - name: out.inter.default.svc.cluster.local|http|version=v1
              weight: 90
              request_headers_to_add:
              - key: x-destination-cluster
                value: out.httpbin.default.svc.cluster.local|http|version=50
            - name: out.inter.default.svc.cluster.local|http|version=v1
              weight: 10
              request_headers_to_add:
              - key: x-destination-cluster
                value: out.httpbin.default.svc.cluster.local|http|version=51

Here is a link to the istio doc.
https://goo.gl/wKNf87

[mandarjog]

@htuch

[mattklein123]

@zuercher can you take a look at ^?

[htuch]

The other alternative is to put metadata in the weighted cluster (not for match but for indicating header values), then refer to that in the standard route "requests headers to add" via a %var%.

[zuercher]

So, if you add that header value as EDS metadata on the upstream hosts, you can add a route-level header with value %UPSTREAM_METADATA(["namespace", "metadata-key"])% (substitute the correct namespace and key) and it'll work with the current version. (This is a slightly more concrete example of what @htuch said.)

Once #2409 lands, you could have just the version in the metadata and set the value to out.httpbin.default.svc.cluster.local|http|version=%UPSTREAM_METADATA(["namespace", "metadata-key"])%

There are use cases for doing this with static metadata. We actually have a feature in the old version of our product that we can't easily reproduce with Envoy unless we add this feature.

[htuch]

This is much cleaner if it works for the Via feature in Istio. I think the doc is saying we have Envoy X that wants to speak to cluster Z. However, we want to allow Istio to inject an intermediary and bounce to Z via cluster Y, so we have X -> Y -> Z. When we send to Y, we need to convey Z in a header to Y to allow it to do the Y -> Z hop.

In the above situation, X only has cluster Y defined and endpoints for Y. Y is likely used by multiple routes that need a Via, for example there might also be X -> Y -> W happening.

Can we put both the metadata for Z and W in Y's endpoint definitions? Maybe if we introduce duplicate endpoint entries for Y and use subset selection, but this is maybe a bit complicated.

Anyway, maybe @mandarjog can evaluate if EDS metadata is sufficient.

[mandarjog]

Using metadata is fine, however EDS metadata associated with the upstream host is one level deeper than the required abstraction.

I would like to associate meta data with {route, chosen cluster}. This is recording a specific use of the cluster, so I don't think it can be a property of the cluster or its endpoints.
Am I missing something?

[zuercher]

I think I misunderstood what the version applied to (the cluster, not the software running on the upstreams). It's possible to introduce a new % expression to access the cluster metadata, but not currently implemented.

[mandarjog]

I think the specific options are

  message ClusterWeight {
     // Name of the upstream cluster. The cluster must exist in the
     // :ref:`cluster manager configuration <config_cluster_manager>`.
     string name = 1 [(validate.rules).string.min_bytes = 1];
 
     // An integer between 0-100. When a request matches the route, the choice of
     // an upstream cluster is determined by its weight. The sum of weights
     // across all entries in the clusters array must add up to 100.
     google.protobuf.UInt32Value weight = 2;
 
     // Optional endpoint metadata match criteria. Only endpoints in the upstream
     // cluster with metadata matching that set in metadata_match will be
     // considered. The filter name should be specified as *envoy.lb*.
     Metadata metadata_match = 3;

    +  // Optional headers to add when this cluster is selected.
    + repeated HeaderValueOption request_headers_to_add = 4;
   }

OR --->
2 )

  message ClusterWeight {
     // Name of the upstream cluster. The cluster must exist in the
     // :ref:`cluster manager configuration <config_cluster_manager>`.
     string name = 1 [(validate.rules).string.min_bytes = 1];
 
     // An integer between 0-100. When a request matches the route, the choice of
     // an upstream cluster is determined by its weight. The sum of weights
     // across all entries in the clusters array must add up to 100.
     google.protobuf.UInt32Value weight = 2;
 
     // Optional endpoint metadata match criteria. Only endpoints in the upstream
     // cluster with metadata matching that set in metadata_match will be
     // considered. The filter name should be specified as *envoy.lb*.
     Metadata metadata_match = 3;

     + // Metadata associated with this route-cluster association.
     + Metadata metadata = 4;
   }

And then introduce new % expression. I think option 1) is more straight forward.

[zuercher]

For option 2, I meant introducing a % expression that pulls from metadata in the CDS Cluster object.

I'm fine with option 1, though.

[mandarjog]

Ok great, I will send a pr in the data plane api repo first.

[rshriram]

@htuch with regard to this: #2455 (comment)

Y knows the destination by looking at the host header. It knows the caller from x-downstream-cluster or something. So, programming Y should be no different from programming X/W. Does it need weight level metadata customization? Shouldn't the existing information be sufficient?

[zuercher]

@mandarjog are you still working on this?

[zuercher]

@mandarjog if I don’t hear otherwise, I’m going to start working on this tomorrow.

[mandarjog]

Hey sorry. Yes It would be great if you start. Does not look like I will be able to get to it soon.