peering: ensure that merged central configs of peered upstreams for partitioned downstreams work #17179
Conversation
| psn.ServiceName.EnterpriseMeta.Normalize() | ||
|
|
||
| // Normalize the partition field specially. | ||
| if psn.Peer != "" { |
There was a problem hiding this comment.
A peered upstream MUST exist in the same partition as the downstream using it. When it is empty, inferring it as default is incorrect.
| @@ -192,6 +206,12 @@ func MergeServiceConfig(defaults *structs.ServiceConfigResponse, service *struct | |||
| } | |||
|
|
|||
| uid := us.DestinationID() | |||
|
|
|||
| // Normalize the partition field specially. | |||
| if uid.Peer != "" { | |||
There was a problem hiding this comment.
Since the write earlier and the read here are now aligned on normalization, there will actually be map key hits now.
| } | ||
| } | ||
|
|
||
| for _, partition := range partitions { |
There was a problem hiding this comment.
The real method call was instrumented with a printf that printed the inputs/outputs to stdout.
One setup was done using client agents and the other using dataplanes and stdout capture was used to create these realistic input/outputs to showcase the shape things were in at this specific point.
| LocalBindAddress: "0.0.0.0", | ||
| LocalBindPort: 5000, | ||
| MeshGateway: structs.MeshGatewayConfig{ | ||
| Mode: "local", // This field vanishes if the merging does not work for dataplanes. |
There was a problem hiding this comment.
This was the only detectable divergence given these inputs.
rboyer
changed the title
…ams for partitioned downstreams work Backport of #17179 into release/1.15.x
rboyer
added
pr/no-backport
and removed
backport/1.15
Description
Partitioned downstreams with peered upstreams could not properly merge central config info (i.e.
proxy-defaultsandservice-defaultsthings like mesh gateway modes) if the upstream had an emptyDestinationPartitionfield in Enterprise.Due to data flow, if this setup is done using Consul client agents the field is never empty and thus does not experience the bug.
When a service is registered directly to the catalog as is the case for
consul-dataplaneuse this field may be empty and and the internal machinery of the merging function doesn't handle this well.This PR ensure the internal machinery of that function is referentially self-consistent.
The easy way to observe the breakage is arrange for:
consul-dataplaneDestinationPartitionfield emptyproxy-defaultswithmeshgateway.mode="local"After bringing everything up, the cluster to reach the upstream from the calling sidecar should point to the local cluster's mesh gateway address (local mode) and not the remote peer's mesh gateway (remote mode).
Needs manual backporting to at least 1.15 (1.14 is very different in this area and may not be applicable).