connect: simplify the compiled discovery chain data structures #6242
Conversation
rboyer
added
the
theme/connect
| var buf bytes.Buffer | ||
| buf.WriteString(url.QueryEscape(t.Service)) | ||
| buf.WriteRune(',') | ||
| buf.WriteString(url.QueryEscape(t.ServiceSubset)) | ||
| buf.WriteString(url.QueryEscape(t.ServiceSubset)) // TODO(rb): move this first so the scoping flows from small->large? |
There was a problem hiding this comment.
If this is empty we end up with something like web,,dc1 or web,,namespace,dc1. Is there a reason to make namespace be optional but not subset? I'd expect subset empty to be 99.9% of cases.
Is there an argument to format this as subset.name.namespace.dc to match SNI or reverse that to match natural precedence dc/namespace/name/subset or something?
All bikesheds as none really affect the behaviour just seems we should do one that makes sense. Is there a specific reason it's URI encoded?
There was a problem hiding this comment.
The query encoding was because I wasn't entirely sure what was and was not a reserved character for 3 of the 4 components (I know I validated subsets).
This encoding predates the bulk envoy naming SNI changes. And given no escaping happens there already then there's no point of escaping stuff here. I actually was thinking of switching the DiscoveryTarget marshalling over to the SNI variant already.
There was a problem hiding this comment.
Going to handle this in #6248 instead.
rboyer
force-pushed
the
upstreams-in-chain
branch
from
1e0f33b to
e888cf7
Compare
rboyer
force-pushed
the
simplifed-compiled-discovery-chain
branch
from
5b246f0 to
f66c962
Compare
This should make them better for sending over RPC or the API. Instead of a chain implemented explicitly like a linked list (nodes holding pointers to other nodes) instead switch to a flat map of named nodes with nodes linking other other nodes by name. The shipped structure is just a map and a string to indicate which key to start from. Other changes: * inline the compiler option InferDefaults as true * introduce compiled target config to avoid needing to send back additional maps of Resolvers; future target-specific compiled state can go here * move compiled MeshGateway out of the Resolver and into the TargetConfig where it makes more sense.
rboyer
force-pushed
the
simplifed-compiled-discovery-chain
branch
from
f66c962 to
a8e1888
Compare
This should make them better for sending over RPC or the API.
Instead of a chain implemented explicitly like a linked list (nodes
holding pointers to other nodes) instead switch to a flat map of named
nodes with nodes linking other other nodes by name. The shipped
structure is just a map and a string to indicate which key to start
from.
Other changes:
inline the compiler option InferDefaults as true
introduce compiled target config to avoid needing to send back
additional maps of Resolvers; future target-specific compiled state
can go here
move compiled MeshGateway out of the Resolver and into the
TargetConfig where it makes more sense.