Document aggregation keyer rules#110
Conversation
Signed-off-by: Jess Yuen <jyuen@lyft.com>
| ``` | ||
| This example specifies that if the DiscoveryRequest type matches LDS or CDS, to generate the first | ||
| fragment from the DiscoveryRequest node ID field by performing a regex replace operation using the | ||
| pattern: `"^(.*)-*$"`, and replacing it with the first regex group match `"$1"`. More concretely, |
There was a problem hiding this comment.
nit: should be "^-(.)-*$"
There was a problem hiding this comment.
can you also define field here? does that correspond to "$1"?
| ). A rule defines how to generate a fragment from an xDS [DiscoveryRequest](https://www.envoyproxy.io/docs/envoy/latest/api-v2/api/v2/discovery.proto#discoveryrequest) | ||
| . The first matching rule applies, and the rest are ignored. | ||
|
|
||
| A rule is formed from a `match` and a `result`. `match` takes a DiscoveryRequest and matches on |
There was a problem hiding this comment.
Should we enclose DiscoveryRequest in backticks? Or maybe link it to the envoy docs (like we're doing in line 39)?
| - "type.googleapis.com/envoy.api.v2.Cluster" | ||
| result: | ||
| request_node_fragment: | ||
| field: 1 |
There was a problem hiding this comment.
We should clarify what those fields are about. They map to the enum values defined in https://github.com/envoyproxy/xds-relay/blob/master/api/protos/aggregation/v1/aggregation.proto#L35-L41. edit: I saw that we specify the possible values in a section below.
As an aside: now that I read it again, we should find a way to use intelligible names instead of the enum values. Not in this PR obviously. Also, why didn't we name this field type? :-)
|
|
||
| #### Match Predicates | ||
| xds-relay current support a limited set of match predicates for rules. More will be added in the | ||
| future on a as needed basis. Refer to the [API protos](https://github.com/envoyproxy/xds-relay/blob/master/api/protos/aggregation/v1/aggregation.proto) |
There was a problem hiding this comment.
typo: "on an as-needed..."
| for the source of truth. | ||
|
|
||
| ##### `request_type_match` | ||
| - Match on the string value of the xDS request type URL. |
There was a problem hiding this comment.
why use lists in the description?
|
|
||
| * [Contributing guide](CONTRIBUTING.md) | ||
|
|
||
| ## Design |
There was a problem hiding this comment.
is this supposed to be a TODO?
|
|
||
| At a high level, an aggregation rule is formed from `fragments`, `rules`, `match`, and `result`s. | ||
|
|
||
| #### Fragments |
There was a problem hiding this comment.
Why straight to heading level 4?
| field: 0 | ||
| exact_match: "canary" | ||
| ``` | ||
|
|
| for the source of truth. | ||
|
|
||
| ##### Actions | ||
| - Actions can be either a `regex_action` or `exact`. Exact uses the result predicate as is, and |
There was a problem hiding this comment.
How about we don't use lists? It is a different style than the first sections of this document.
| #### Rules | ||
| Each `fragment` can have multiple `rule`s ([example](https://github.com/envoyproxy/xds-relay/blob/c83a956952c3e9762a08420ffc1c6bda5e0fff04/integration/testdata/keyer_configuration_e2e.yaml#L28-L52) | ||
| ). A rule defines how to generate a fragment from an xDS [DiscoveryRequest](https://www.envoyproxy.io/docs/envoy/latest/api-v2/api/v2/discovery.proto#discoveryrequest) | ||
| . The first matching rule applies, and the rest are ignored. |
There was a problem hiding this comment.
I don't think this is true, i.e. we can have multiple matches in a list of rules. The mapper currently matches fragments for all rules, not only the first one (as this comment implies).
| differ greatly between clusters. For example, a service application scaled across 100 hosts will | ||
| likely require the same data plane configurations across all hosts. | ||
|
|
||
| At a high level, an aggregation rule is formed from `fragments`, `rules`, `match`, and `result`s. |
There was a problem hiding this comment.
We should give a better description of how each one of these 4 components interact. For example, aggregations rules are composed of fragments, and a single fragment is composed of a list of rules where and each rule is composed of a pair composed of a match predicate and a result predicate.
Do you think it's too much to try to specify this using EBNF? It might be, I'm just wondering if having that could alleviate the problem of being total in this document.
3 participants
Signed-off-by: Jess Yuen jyuen@lyft.com