From 986bcccbeae8595cc5fd19dd14488a35050413f6 Mon Sep 17 00:00:00 2001 From: freddygv Date: Fri, 19 Mar 2021 22:03:17 -0600 Subject: [PATCH] Pass down upstream defaults to client proxies This is needed in case the client proxy is in TransparentProxy mode. Typically they won't have explicit configuration for every upstream, so this ensures the settings can be applied to all of them when generating xDS config. --- agent/consul/config_endpoint.go | 29 +++++- agent/consul/config_endpoint_test.go | 146 +++++++++++++++++++++++++++ agent/service_manager.go | 17 ++-- agent/structs/config_entry.go | 3 + 4 files changed, 184 insertions(+), 11 deletions(-) diff --git a/agent/consul/config_endpoint.go b/agent/consul/config_endpoint.go index a3f39f7b11..2b3e345e54 100644 --- a/agent/consul/config_endpoint.go +++ b/agent/consul/config_endpoint.go @@ -410,6 +410,21 @@ func (c *ConfigEntry) ResolveServiceConfig(args *structs.ServiceConfigRequest, r upstreamIDs := args.UpstreamIDs legacyUpstreams := false + var ( + noUpstreamArgs = len(upstreamIDs) == 0 && len(args.Upstreams) == 0 + + // Check the args and the resolved value. If it was exclusively set via a config entry, then args.TransparentProxy + // will never be true because the service config request does not use the resolved value. + tproxy = args.TransparentProxy || reply.TransparentProxy + ) + + // The upstreams passed as arguments to this endpoint are the upstreams explicitly defined in a proxy registration. + // If no upstreams were passed, then we should only returned the resolved config if the proxy has TransparentProxy mode enabled. + // Otherwise we would return a resolved upstream config to a proxy with no configured upstreams. + if noUpstreamArgs && !tproxy { + return nil + } + // The request is considered legacy if the deprecated args.Upstream was used if len(upstreamIDs) == 0 && len(args.Upstreams) > 0 { legacyUpstreams = true @@ -437,6 +452,9 @@ func (c *ConfigEntry) ResolveServiceConfig(args *structs.ServiceConfigRequest, r } } + // usConfigs stores the opaque config map for each upstream and is keyed on the upstream's ID. + usConfigs := make(map[structs.ServiceID]map[string]interface{}) + var ( upstreamDefaults *structs.UpstreamConfig upstreamConfigs map[string]*structs.UpstreamConfig @@ -444,15 +462,20 @@ func (c *ConfigEntry) ResolveServiceConfig(args *structs.ServiceConfigRequest, r if serviceConf != nil && serviceConf.Connect != nil { if serviceConf.Connect.UpstreamDefaults != nil { upstreamDefaults = serviceConf.Connect.UpstreamDefaults + + // Store the upstream defaults under a wildcard key so that they can be applied to + // upstreams that are inferred from intentions and do not have explicit upstream configuration. + cfgMap := make(map[string]interface{}) + upstreamDefaults.MergeInto(cfgMap) + + wildcard := structs.NewServiceID(structs.WildcardSpecifier, structs.WildcardEnterpriseMeta()) + usConfigs[wildcard] = cfgMap } if serviceConf.Connect.UpstreamConfigs != nil { upstreamConfigs = serviceConf.Connect.UpstreamConfigs } } - // usConfigs stores the opaque config map for each upstream and is keyed on the upstream's ID. - usConfigs := make(map[structs.ServiceID]map[string]interface{}) - for upstream := range seenUpstreams { resolvedCfg := make(map[string]interface{}) diff --git a/agent/consul/config_endpoint_test.go b/agent/consul/config_endpoint_test.go index 188aab44f9..ea17042011 100644 --- a/agent/consul/config_endpoint_test.go +++ b/agent/consul/config_endpoint_test.go @@ -1000,6 +1000,7 @@ func TestConfigEntry_ResolveServiceConfig_Upstreams(t *testing.T) { mysql := structs.NewServiceID("mysql", structs.DefaultEnterpriseMeta()) cache := structs.NewServiceID("cache", structs.DefaultEnterpriseMeta()) + wildcard := structs.NewServiceID(structs.WildcardSpecifier, structs.WildcardEnterpriseMeta()) tt := []struct { name string @@ -1126,6 +1127,14 @@ func TestConfigEntry_ResolveServiceConfig_Upstreams(t *testing.T) { }, expect: structs.ServiceConfigResponse{ UpstreamIDConfigs: structs.OpaqueUpstreamConfigs{ + { + Upstream: wildcard, + Config: map[string]interface{}{ + "mesh_gateway": map[string]interface{}{ + "Mode": "remote", + }, + }, + }, { Upstream: mysql, Config: map[string]interface{}{ @@ -1188,6 +1197,19 @@ func TestConfigEntry_ResolveServiceConfig_Upstreams(t *testing.T) { "protocol": "udp", }, UpstreamIDConfigs: structs.OpaqueUpstreamConfigs{ + { + Upstream: wildcard, + Config: map[string]interface{}{ + "passive_health_check": map[string]interface{}{ + "Interval": int64(10), + "MaxFailures": int64(2), + }, + "mesh_gateway": map[string]interface{}{ + "Mode": "remote", + }, + "protocol": "http", + }, + }, { Upstream: mysql, Config: map[string]interface{}{ @@ -1204,6 +1226,130 @@ func TestConfigEntry_ResolveServiceConfig_Upstreams(t *testing.T) { }, }, }, + { + name: "without upstream args we should return centralized config with tproxy arg", + entries: []structs.ConfigEntry{ + &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: "api", + Connect: &structs.ConnectConfiguration{ + UpstreamDefaults: &structs.UpstreamConfig{ + MeshGateway: structs.MeshGatewayConfig{Mode: structs.MeshGatewayModeRemote}, + }, + UpstreamConfigs: map[string]*structs.UpstreamConfig{ + mysql.String(): { + Protocol: "grpc", + }, + }, + }, + }, + }, + request: structs.ServiceConfigRequest{ + Name: "api", + Datacenter: "dc1", + TransparentProxy: true, + + // Empty Upstreams/UpstreamIDs + }, + expect: structs.ServiceConfigResponse{ + UpstreamIDConfigs: structs.OpaqueUpstreamConfigs{ + { + Upstream: wildcard, + Config: map[string]interface{}{ + "mesh_gateway": map[string]interface{}{ + "Mode": "remote", + }, + }, + }, + { + Upstream: mysql, + Config: map[string]interface{}{ + "protocol": "grpc", + "mesh_gateway": map[string]interface{}{ + "Mode": "remote", + }, + }, + }, + }, + }, + }, + { + name: "without upstream args we should return centralized config with tproxy default", + entries: []structs.ConfigEntry{ + &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: "api", + Connect: &structs.ConnectConfiguration{ + UpstreamDefaults: &structs.UpstreamConfig{ + MeshGateway: structs.MeshGatewayConfig{Mode: structs.MeshGatewayModeRemote}, + }, + UpstreamConfigs: map[string]*structs.UpstreamConfig{ + mysql.String(): { + Protocol: "grpc", + }, + }, + }, + + // TransparentProxy on the config entry but not the config request + TransparentProxy: true, + }, + }, + request: structs.ServiceConfigRequest{ + Name: "api", + Datacenter: "dc1", + + // Empty Upstreams/UpstreamIDs + }, + expect: structs.ServiceConfigResponse{ + TransparentProxy: true, + UpstreamIDConfigs: structs.OpaqueUpstreamConfigs{ + { + Upstream: wildcard, + Config: map[string]interface{}{ + "mesh_gateway": map[string]interface{}{ + "Mode": "remote", + }, + }, + }, + { + Upstream: mysql, + Config: map[string]interface{}{ + "protocol": "grpc", + "mesh_gateway": map[string]interface{}{ + "Mode": "remote", + }, + }, + }, + }, + }, + }, + { + name: "without upstream args we should NOT return centralized config outside tproxy mode", + entries: []structs.ConfigEntry{ + &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: "api", + Connect: &structs.ConnectConfiguration{ + UpstreamDefaults: &structs.UpstreamConfig{ + MeshGateway: structs.MeshGatewayConfig{Mode: structs.MeshGatewayModeRemote}, + }, + UpstreamConfigs: map[string]*structs.UpstreamConfig{ + mysql.String(): { + Protocol: "grpc", + }, + }, + }, + }, + }, + request: structs.ServiceConfigRequest{ + Name: "api", + Datacenter: "dc1", + TransparentProxy: false, + + // Empty Upstreams/UpstreamIDs + }, + expect: structs.ServiceConfigResponse{}, + }, } for _, tc := range tt { diff --git a/agent/service_manager.go b/agent/service_manager.go index f45d24c328..d48ba26326 100644 --- a/agent/service_manager.go +++ b/agent/service_manager.go @@ -335,12 +335,13 @@ func makeConfigRequest(bd BaseDeps, addReq AddServiceRequest) *structs.ServiceCo } req := &structs.ServiceConfigRequest{ - Name: name, - Datacenter: bd.RuntimeConfig.Datacenter, - QueryOptions: structs.QueryOptions{Token: addReq.token}, - MeshGateway: ns.Proxy.MeshGateway, - UpstreamIDs: upstreams, - EnterpriseMeta: ns.EnterpriseMeta, + Name: name, + Datacenter: bd.RuntimeConfig.Datacenter, + QueryOptions: structs.QueryOptions{Token: addReq.token}, + MeshGateway: ns.Proxy.MeshGateway, + TransparentProxy: ns.Proxy.TransparentProxy, + UpstreamIDs: upstreams, + EnterpriseMeta: ns.EnterpriseMeta, } if req.QueryOptions.Token == "" { req.QueryOptions.Token = bd.Tokens.AgentToken() @@ -436,8 +437,8 @@ func mergeServiceConfig(defaults *structs.ServiceConfigResponse, service *struct } // Ensure upstreams present in central config are represented in the local configuration. - // This does not apply outside of TransparentProxy mode because in that situation every upstream needs to be defined - // explicitly and locally with a local bind port. + // This does not apply outside of TransparentProxy mode because in that situation every possible upstream already exists + // inside of ns.Proxy.Upstreams. if ns.Proxy.TransparentProxy { for id, remote := range remoteUpstreams { if _, ok := localUpstreams[id]; ok { diff --git a/agent/structs/config_entry.go b/agent/structs/config_entry.go index c28718fd2f..4c14d5005d 100644 --- a/agent/structs/config_entry.go +++ b/agent/structs/config_entry.go @@ -590,6 +590,9 @@ type ServiceConfigRequest struct { // MeshGateway contains the mesh gateway configuration from the requesting proxy's registration MeshGateway MeshGatewayConfig + // TransparentProxy indicates whether the requesting proxy is in transparent proxy mode + TransparentProxy bool + UpstreamIDs []ServiceID // DEPRECATED