Merge central config for GetEnvoyBootstrapParams (#14869)

This fixes GetEnvoyBootstrapParams to merge in proxy-defaults and service-defaults.

Co-authored-by: Dan Upton <daniel@floppy.co>
This commit is contained in:
Paul Glass 2022-10-10 12:40:27 -05:00 committed by GitHub
parent b757624b59
commit c0c187f1c5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 136 additions and 30 deletions

3
.changelog/14869.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
grpc: Merge proxy-defaults and service-defaults in GetEnvoyBootstrapParams response.
```

View File

@ -1,4 +1,4 @@
package consul
package configentry
import (
"fmt"
@ -8,18 +8,21 @@ import (
"github.com/imdario/mergo"
"github.com/mitchellh/copystructure"
"github.com/hashicorp/consul/agent/configentry"
"github.com/hashicorp/consul/agent/consul/state"
"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/agent/structs"
)
// mergeNodeServiceWithCentralConfig merges a service instance (NodeService) with the
type StateStore interface {
ReadResolvedServiceConfigEntries(memdb.WatchSet, string, *acl.EnterpriseMeta, []structs.ServiceID, structs.ProxyMode) (uint64, *ResolvedServiceConfigSet, error)
}
// MergeNodeServiceWithCentralConfig merges a service instance (NodeService) with the
// proxy-defaults/global and service-defaults/:service config entries.
// This common helper is used by the blocking query function of different RPC endpoints
// that need to return a fully resolved service defintion.
func mergeNodeServiceWithCentralConfig(
func MergeNodeServiceWithCentralConfig(
ws memdb.WatchSet,
state *state.Store,
state StateStore,
args *structs.ServiceSpecificRequest,
ns *structs.NodeService,
logger hclog.Logger) (uint64, *structs.NodeService, error) {
@ -67,7 +70,7 @@ func mergeNodeServiceWithCentralConfig(
ns.ID, err)
}
defaults, err := configentry.ComputeResolvedServiceConfig(
defaults, err := ComputeResolvedServiceConfig(
configReq,
upstreams,
false,

View File

@ -1,4 +1,4 @@
package consul
package configentry
import (
"testing"

View File

@ -16,6 +16,7 @@ import (
"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/acl/resolver"
"github.com/hashicorp/consul/agent/configentry"
"github.com/hashicorp/consul/agent/consul/state"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/ipaddr"
@ -752,7 +753,7 @@ func (c *Catalog) ServiceNodes(args *structs.ServiceSpecificRequest, reply *stru
mergedsn := sn
ns := sn.ToNodeService()
if ns.IsSidecarProxy() || ns.IsGateway() {
cfgIndex, mergedns, err := mergeNodeServiceWithCentralConfig(ws, state, args, ns, c.logger)
cfgIndex, mergedns, err := configentry.MergeNodeServiceWithCentralConfig(ws, state, args, ns, c.logger)
if err != nil {
return err
}
@ -960,7 +961,7 @@ func (c *Catalog) NodeServiceList(args *structs.NodeSpecificRequest, reply *stru
Datacenter: args.Datacenter,
QueryOptions: args.QueryOptions,
}
cfgIndex, mergedns, err = mergeNodeServiceWithCentralConfig(ws, state, &serviceSpecificReq, ns, c.logger)
cfgIndex, mergedns, err = configentry.MergeNodeServiceWithCentralConfig(ws, state, &serviceSpecificReq, ns, c.logger)
if err != nil {
return err
}

View File

@ -11,6 +11,7 @@ import (
hashstructure_v2 "github.com/mitchellh/hashstructure/v2"
"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/agent/configentry"
"github.com/hashicorp/consul/agent/consul/state"
"github.com/hashicorp/consul/agent/structs"
)
@ -256,7 +257,7 @@ func (h *Health) ServiceNodes(args *structs.ServiceSpecificRequest, reply *struc
for _, node := range resolvedNodes {
ns := node.Service
if ns.IsSidecarProxy() || ns.IsGateway() {
cfgIndex, mergedns, err := mergeNodeServiceWithCentralConfig(ws, state, args, ns, h.logger)
cfgIndex, mergedns, err := configentry.MergeNodeServiceWithCentralConfig(ws, state, args, ns, h.logger)
if err != nil {
return err
}

View File

@ -9,10 +9,11 @@ import (
"google.golang.org/grpc/status"
"google.golang.org/protobuf/types/known/structpb"
acl "github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/agent/configentry"
"github.com/hashicorp/consul/agent/consul/state"
external "github.com/hashicorp/consul/agent/grpc-external"
structs "github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/proto-public/pbdataplane"
)
@ -73,7 +74,24 @@ func (s *Server) GetEnvoyBootstrapParams(ctx context.Context, req *pbdataplane.G
NodeId: string(svc.ID),
}
bootstrapConfig, err := structpb.NewStruct(svc.ServiceProxy.Config)
// This is awkward because it's designed for different requests, but
// this fakes the ServiceSpecificRequest so that we can reuse code.
_, ns, err := configentry.MergeNodeServiceWithCentralConfig(
nil,
store,
&structs.ServiceSpecificRequest{
Datacenter: s.Datacenter,
QueryOptions: options,
},
svc.ToNodeService(),
logger,
)
if err != nil {
logger.Error("Error merging with central config", "error", err)
return nil, status.Errorf(codes.Unknown, "Error merging central config: %v", err)
}
bootstrapConfig, err := structpb.NewStruct(ns.Proxy.Config)
if err != nil {
logger.Error("Error creating the envoy boostrap params config", "error", err)
return nil, status.Error(codes.Unknown, "Error creating the envoy boostrap params config")

View File

@ -5,29 +5,39 @@ import (
"testing"
"github.com/hashicorp/go-hclog"
mock "github.com/stretchr/testify/mock"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/types/known/structpb"
acl "github.com/hashicorp/consul/acl"
resolver "github.com/hashicorp/consul/acl/resolver"
"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/acl/resolver"
external "github.com/hashicorp/consul/agent/grpc-external"
"github.com/hashicorp/consul/agent/grpc-external/testutils"
structs "github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/proto-public/pbdataplane"
"github.com/hashicorp/consul/types"
)
const (
testToken = "acl-token-get-envoy-bootstrap-params"
testServiceName = "web"
proxyServiceID = "web-proxy"
nodeName = "foo"
nodeID = "2980b72b-bd9d-9d7b-d4f9-951bf7508d95"
proxyConfigKey = "envoy_dogstatsd_url"
proxyConfigValue = "udp://127.0.0.1:8125"
serverDC = "dc1"
protocolKey = "protocol"
connectTimeoutKey = "local_connect_timeout_ms"
requestTimeoutKey = "local_request_timeout_ms"
proxyDefaultsProtocol = "http"
proxyDefaultsRequestTimeout = 1111
serviceDefaultsProtocol = "tcp"
serviceDefaultsConnectTimeout = 4444
)
func testRegisterRequestProxy(t *testing.T) *structs.RegisterRequest {
@ -43,7 +53,7 @@ func testRegisterRequestProxy(t *testing.T) *structs.RegisterRequest {
Address: "127.0.0.2",
Port: 2222,
Proxy: structs.ConnectProxyConfig{
DestinationServiceName: "web",
DestinationServiceName: testServiceName,
Config: map[string]interface{}{
proxyConfigKey: proxyConfigValue,
},
@ -63,18 +73,57 @@ func testRegisterIngressGateway(t *testing.T) *structs.RegisterRequest {
return registerReq
}
func testProxyDefaults(t *testing.T) structs.ConfigEntry {
return &structs.ProxyConfigEntry{
Kind: structs.ProxyDefaults,
Name: structs.ProxyConfigGlobal,
Config: map[string]interface{}{
protocolKey: proxyDefaultsProtocol,
requestTimeoutKey: proxyDefaultsRequestTimeout,
},
}
}
func testServiceDefaults(t *testing.T) structs.ConfigEntry {
return &structs.ServiceConfigEntry{
Kind: structs.ServiceDefaults,
Name: testServiceName,
Protocol: serviceDefaultsProtocol,
LocalConnectTimeoutMs: serviceDefaultsConnectTimeout,
}
}
func requireConfigField(t *testing.T, resp *pbdataplane.GetEnvoyBootstrapParamsResponse, key string, value interface{}) {
require.Contains(t, resp.Config.Fields, key)
require.Equal(t, value, resp.Config.Fields[key])
}
func TestGetEnvoyBootstrapParams_Success(t *testing.T) {
type testCase struct {
name string
registerReq *structs.RegisterRequest
nodeID bool
proxyDefaults structs.ConfigEntry
serviceDefaults structs.ConfigEntry
}
run := func(t *testing.T, tc testCase) {
store := testutils.TestStateStore(t, nil)
err := store.EnsureRegistration(1, tc.registerReq)
idx := uint64(1)
err := store.EnsureRegistration(idx, tc.registerReq)
require.NoError(t, err)
if tc.proxyDefaults != nil {
idx++
err := store.EnsureConfigEntry(idx, tc.proxyDefaults)
require.NoError(t, err)
}
if tc.serviceDefaults != nil {
idx++
err := store.EnsureConfigEntry(idx, tc.serviceDefaults)
require.NoError(t, err)
}
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", testToken, mock.Anything, mock.Anything).
Return(testutils.TestAuthorizerServiceRead(t, tc.registerReq.Service.ID), nil)
@ -109,20 +158,33 @@ func TestGetEnvoyBootstrapParams_Success(t *testing.T) {
require.Equal(t, serverDC, resp.Datacenter)
require.Equal(t, tc.registerReq.EnterpriseMeta.PartitionOrDefault(), resp.Partition)
require.Equal(t, tc.registerReq.EnterpriseMeta.NamespaceOrDefault(), resp.Namespace)
require.Contains(t, resp.Config.Fields, proxyConfigKey)
require.Equal(t, structpb.NewStringValue(proxyConfigValue), resp.Config.Fields[proxyConfigKey])
requireConfigField(t, resp, proxyConfigKey, structpb.NewStringValue(proxyConfigValue))
require.Equal(t, convertToResponseServiceKind(tc.registerReq.Service.Kind), resp.ServiceKind)
require.Equal(t, tc.registerReq.Node, resp.NodeName)
require.Equal(t, string(tc.registerReq.ID), resp.NodeId)
if tc.serviceDefaults != nil && tc.proxyDefaults != nil {
// service-defaults take precedence over proxy-defaults
requireConfigField(t, resp, protocolKey, structpb.NewStringValue(serviceDefaultsProtocol))
requireConfigField(t, resp, connectTimeoutKey, structpb.NewNumberValue(serviceDefaultsConnectTimeout))
requireConfigField(t, resp, requestTimeoutKey, structpb.NewNumberValue(proxyDefaultsRequestTimeout))
} else if tc.serviceDefaults != nil {
requireConfigField(t, resp, protocolKey, structpb.NewStringValue(serviceDefaultsProtocol))
requireConfigField(t, resp, connectTimeoutKey, structpb.NewNumberValue(serviceDefaultsConnectTimeout))
} else if tc.proxyDefaults != nil {
requireConfigField(t, resp, protocolKey, structpb.NewStringValue(proxyDefaultsProtocol))
requireConfigField(t, resp, requestTimeoutKey, structpb.NewNumberValue(proxyDefaultsRequestTimeout))
}
}
testCases := []testCase{
{
name: "lookup service side car proxy by node name",
name: "lookup service sidecar proxy by node name",
registerReq: testRegisterRequestProxy(t),
},
{
name: "lookup service side car proxy by node ID",
name: "lookup service sidecar proxy by node ID",
registerReq: testRegisterRequestProxy(t),
nodeID: true,
},
@ -135,6 +197,21 @@ func TestGetEnvoyBootstrapParams_Success(t *testing.T) {
registerReq: testRegisterIngressGateway(t),
nodeID: true,
},
{
name: "merge proxy defaults for sidecar proxy",
registerReq: testRegisterRequestProxy(t),
proxyDefaults: testProxyDefaults(t),
},
{
name: "merge service defaults for sidecar proxy",
registerReq: testRegisterRequestProxy(t),
serviceDefaults: testServiceDefaults(t),
},
{
name: "merge proxy defaults and service defaults for sidecar proxy",
registerReq: testRegisterRequestProxy(t),
serviceDefaults: testServiceDefaults(t),
},
}
for _, tc := range testCases {

View File

@ -4,9 +4,11 @@ import (
"google.golang.org/grpc"
"github.com/hashicorp/go-hclog"
"github.com/hashicorp/go-memdb"
"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/acl/resolver"
"github.com/hashicorp/consul/agent/configentry"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/proto-public/pbdataplane"
)
@ -25,6 +27,7 @@ type Config struct {
type StateStore interface {
ServiceNode(string, string, string, *acl.EnterpriseMeta, string) (uint64, *structs.ServiceNode, error)
ReadResolvedServiceConfigEntries(memdb.WatchSet, string, *acl.EnterpriseMeta, []structs.ServiceID, structs.ProxyMode) (uint64, *configentry.ResolvedServiceConfigSet, error)
}
//go:generate mockery --name ACLResolver --inpackage

View File

@ -8,7 +8,7 @@ import (
"github.com/hashicorp/consul/agent/cache"
cachetype "github.com/hashicorp/consul/agent/cache-types"
"github.com/hashicorp/consul/agent/consul"
"github.com/hashicorp/consul/agent/configentry"
"github.com/hashicorp/consul/agent/structs"
)
@ -145,7 +145,7 @@ func (w *serviceConfigWatch) register(ctx context.Context) error {
// Merge the local registration with the central defaults and update this service
// in the local state.
merged, err := consul.MergeServiceConfig(serviceDefaults, w.registration.Service)
merged, err := configentry.MergeServiceConfig(serviceDefaults, w.registration.Service)
if err != nil {
return err
}
@ -275,7 +275,7 @@ func (w *serviceConfigWatch) handleUpdate(ctx context.Context, event cache.Updat
// Merge the local registration with the central defaults and update this service
// in the local state.
merged, err := consul.MergeServiceConfig(serviceDefaults, w.registration.Service)
merged, err := configentry.MergeServiceConfig(serviceDefaults, w.registration.Service)
if err != nil {
return err
}