connect: generate the full SNI names for discovery targets in the compiler rather than in the xds package #6340
Conversation
rboyer
added
the
theme/connect
|
(Pre-flight comments)
Only on servers still though right? Do we have a helpful error if it's not enabled? If so I think that's fine for now, discovery chain kinda requires Connect for now. If we do extend it to control regular Service Discovery APIs later we might need to review this (i.e. make sure enough of it still works without connect enabled for basic discovery/failover without SNI). |
| @@ -111,7 +112,7 @@ func (s *Server) clustersFromSnapshotMeshGateway(cfgSnap *proxycfg.ConfigSnapsho | |||
| // generate the service subset clusters | |||
| for svc, resolver := range cfgSnap.MeshGateway.ServiceResolvers { | |||
| for subsetName, _ := range resolver.Subsets { | |||
| clusterName := ServiceSNI(svc, subsetName, "default", cfgSnap.Datacenter, cfgSnap) | |||
| clusterName := connect.ServiceSNI(svc, subsetName, "default", cfgSnap.Datacenter, cfgSnap.Roots.TrustDomain) | |||
There was a problem hiding this comment.
Would it be better to take this from the API (new Name field) instead of keeping the duplicated logic? I think that should be available form the target here right?
I don't think it matters too much for now if it's hard for some reason but it seems to be bypassing the new source of truth being the API rather than just shared conventions?
There was a problem hiding this comment.
This is the mesh gateway branch of the xds code that doesn't talk to the discovery chain.
Note this is layered over #6324
One side effect is that you need to enable connect for the
/v1/discovery-chain/endpoint to work.