parse config protocol on write to optimize disco-chain compilation (#…

…19829) * parse config protocol on write to optimize disco-chain compilation * add changelog
hashicorp · Dec 7, 2023 · d93f7f7 · d93f7f7
1 parent bfad6a4
commit d93f7f7
Show file tree

Hide file tree

Showing 13 changed files with 80 additions and 38 deletions.
diff --git a/.changelog/19829.txt b/.changelog/19829.txt
@@ -0,0 +1,3 @@
+```release-note:enhancement
+mesh: parse the proxy-defaults protocol when write the config-entry to avoid parsing it when compiling the discovery chain.
+```
diff --git a/agent/consul/discoverychain/compile.go b/agent/consul/discoverychain/compile.go
@@ -8,14 +8,12 @@ import (
 	"strings"
 	"time"
 
-	"github.com/mitchellh/hashstructure"
-	"github.com/mitchellh/mapstructure"
-
 	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/configentry"
 	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/proto/private/pbpeering"
+	"github.com/mitchellh/hashstructure"
 )
 
 type CompileRequest struct {
@@ -244,21 +242,13 @@ func (c *compiler) recordServiceProtocol(sid structs.ServiceID) error {
 		return c.recordProtocol(sid, serviceDefault.Protocol)
 	}
 	if proxyDefault := c.entries.GetProxyDefaults(sid.PartitionOrDefault()); proxyDefault != nil {
-		var cfg proxyConfig
-		// Ignore errors and fallback on defaults if it does happen.
-		_ = mapstructure.WeakDecode(proxyDefault.Config, &cfg)
-		if cfg.Protocol != "" {
-			return c.recordProtocol(sid, cfg.Protocol)
+		if proxyDefault.Protocol != "" {
+			return c.recordProtocol(sid, proxyDefault.Protocol)
 		}
 	}
 	return c.recordProtocol(sid, "")
 }
 
-// proxyConfig is a snippet from agent/xds/config.go:ProxyConfig
-type proxyConfig struct {
-	Protocol string `mapstructure:"protocol"`
-}
-
 func (c *compiler) recordProtocol(fromService structs.ServiceID, protocol string) error {
 	if protocol == "" {
 		protocol = "tcp"

diff --git a/agent/consul/discoverychain/compile_test.go b/agent/consul/discoverychain/compile_test.go
@@ -1142,8 +1142,9 @@ func testcase_PeerRedirect() compileTestCase {
 func testcase_PeerRedirectProxyDefHTTP() compileTestCase {
 	entries := newEntries()
 	entries.AddProxyDefaults(&structs.ProxyConfigEntry{
-		Kind: structs.ProxyDefaults,
-		Name: structs.ProxyConfigGlobal,
+		Kind:     structs.ProxyDefaults,
+		Name:     structs.ProxyConfigGlobal,
+		Protocol: "http",
 		Config: map[string]interface{}{
 			"Protocol": "http",
 		},
@@ -1768,11 +1769,9 @@ func testcase_DefaultResolver_WithProxyDefaults() compileTestCase {
 	entries := newEntries()
 
 	entries.AddProxyDefaults(&structs.ProxyConfigEntry{
-		Kind: structs.ProxyDefaults,
-		Name: structs.ProxyConfigGlobal,
-		Config: map[string]interface{}{
-			"protocol": "grpc",
-		},
+		Kind:     structs.ProxyDefaults,
+		Name:     structs.ProxyConfigGlobal,
+		Protocol: "grpc",
 		MeshGateway: structs.MeshGatewayConfig{
 			Mode: structs.MeshGatewayModeRemote,
 		},
@@ -3263,8 +3262,9 @@ func newSimpleRoute(name string, muts ...func(*structs.ServiceRoute)) structs.Se
 
 func setGlobalProxyProtocol(entries *configentry.DiscoveryChainSet, protocol string) {
 	entries.AddProxyDefaults(&structs.ProxyConfigEntry{
-		Kind: structs.ProxyDefaults,
-		Name: structs.ProxyConfigGlobal,
+		Kind:     structs.ProxyDefaults,
+		Name:     structs.ProxyConfigGlobal,
+		Protocol: protocol,
 		Config: map[string]interface{}{
 			"protocol": protocol,
 		},

diff --git a/agent/consul/discoverychain/gateway_test.go b/agent/consul/discoverychain/gateway_test.go
@@ -879,8 +879,9 @@ func TestGatewayChainSynthesizer_ComplexChain(t *testing.T) {
 					}},
 				},
 				&structs.ProxyConfigEntry{
-					Kind: structs.ProxyConfigGlobal,
-					Name: "global",
+					Kind:     structs.ProxyConfigGlobal,
+					Name:     "global",
+					Protocol: "http",
 					Config: map[string]interface{}{
 						"protocol": "http",
 					},

diff --git a/agent/consul/state/config_entry.go b/agent/consul/state/config_entry.go
@@ -6,6 +6,7 @@ package state
 import (
 	"errors"
 	"fmt"
+	"github.com/mitchellh/mapstructure"
 	"strings"
 
 	memdb "github.com/hashicorp/go-memdb"
@@ -519,6 +520,11 @@ func insertConfigEntryWithTxn(tx WriteTxn, idx uint64, conf structs.ConfigEntry)
 		if err != nil {
 			return err
 		}
+	case structs.ProxyDefaults:
+		err := addProtocol(conf.(*structs.ProxyConfigEntry))
+		if err != nil {
+			return err
+		}
 	}
 
 	// Assign virtual-ips, if needed
@@ -544,6 +550,21 @@ func insertConfigEntryWithTxn(tx WriteTxn, idx uint64, conf structs.ConfigEntry)
 	return nil
 }
 
+// proxyConfig is a snippet from agent/xds/config.go:ProxyConfig
+type proxyConfig struct {
+	Protocol string `mapstructure:"protocol"`
+}
+
+func addProtocol(conf *structs.ProxyConfigEntry) error {
+	var cfg proxyConfig
+	err := mapstructure.WeakDecode(conf.Config, &cfg)
+	if err != nil {
+		return err
+	}
+	conf.Protocol = cfg.Protocol
+	return nil
+}
+
 func configEntryHasVirtualIP(c structs.ConfigEntry) bool {
 	if c == nil || c.GetName() == "" {
 		return false

diff --git a/agent/consul/state/config_entry_test.go b/agent/consul/state/config_entry_test.go
@@ -24,16 +24,27 @@ import (
 func TestStore_ConfigEntry(t *testing.T) {
 	s := testConfigStateStore(t)
 
+	cfg := &structs.ProxyConfigEntry{
+		Kind: structs.ProxyDefaults,
+		Name: "global",
+		Config: map[string]interface{}{
+			"DestinationServiceName": "foo",
+			"protocol":               "udp",
+		},
+	}
+
 	expected := &structs.ProxyConfigEntry{
 		Kind: structs.ProxyDefaults,
 		Name: "global",
 		Config: map[string]interface{}{
 			"DestinationServiceName": "foo",
+			"protocol":               "udp",
 		},
+		Protocol: "udp",
 	}
 
 	// Create
-	require.NoError(t, s.EnsureConfigEntry(0, expected))
+	require.NoError(t, s.EnsureConfigEntry(0, cfg))
 
 	idx, config, err := s.ConfigEntry(nil, structs.ProxyDefaults, "global", nil)
 	require.NoError(t, err)

diff --git a/agent/proxycfg/manager_test.go b/agent/proxycfg/manager_test.go
@@ -73,8 +73,9 @@ func TestManager_BasicLifecycle(t *testing.T) {
 	dbSplitChain := func() *structs.CompiledDiscoveryChain {
 		set := configentry.NewDiscoveryChainSet()
 		set.AddEntries(&structs.ProxyConfigEntry{
-			Kind: structs.ProxyDefaults,
-			Name: structs.ProxyConfigGlobal,
+			Kind:     structs.ProxyDefaults,
+			Name:     structs.ProxyConfigGlobal,
+			Protocol: "http",
 			Config: map[string]interface{}{
 				"protocol": "http",
 			},

diff --git a/agent/proxycfg/testing_mesh_gateway.go b/agent/proxycfg/testing_mesh_gateway.go
@@ -649,6 +649,7 @@ func TestConfigSnapshotPeeredMeshGateway(t testing.T, variant string, nsFn func(
 		)
 	case "default-services-http":
 		proxyDefaults := &structs.ProxyConfigEntry{
+			Protocol: "http",
 			Config: map[string]interface{}{
 				"protocol": "http",
 			},
@@ -817,8 +818,9 @@ func TestConfigSnapshotPeeredMeshGateway(t testing.T, variant string, nsFn func(
 	case "chain-and-l7-stuff":
 		entries = []structs.ConfigEntry{
 			&structs.ProxyConfigEntry{
-				Kind: structs.ProxyDefaults,
-				Name: structs.ProxyConfigGlobal,
+				Kind:     structs.ProxyDefaults,
+				Name:     structs.ProxyConfigGlobal,
+				Protocol: "http",
 				Config: map[string]interface{}{
 					"protocol": "http",
 				},

diff --git a/agent/proxycfg/testing_tproxy.go b/agent/proxycfg/testing_tproxy.go
@@ -131,8 +131,9 @@ func TestConfigSnapshotTransparentProxy(t testing.T) *ConfigSnapshot {
 func TestConfigSnapshotTransparentProxyHTTPUpstream(t testing.T, additionalEntries ...structs.ConfigEntry) *ConfigSnapshot {
 	// Set default service protocol to HTTP
 	entries := append(additionalEntries, &structs.ProxyConfigEntry{
-		Kind: structs.ProxyDefaults,
-		Name: structs.ProxyConfigGlobal,
+		Kind:     structs.ProxyDefaults,
+		Name:     structs.ProxyConfigGlobal,
+		Protocol: "http",
 		Config: map[string]interface{}{
 			"protocol": "http",
 		},

diff --git a/agent/proxycfg/testing_upstreams.go b/agent/proxycfg/testing_upstreams.go
@@ -481,6 +481,7 @@ func setupTestVariationDiscoveryChain(
 				Kind:           structs.ProxyDefaults,
 				Name:           structs.ProxyConfigGlobal,
 				EnterpriseMeta: em,
+				Protocol:       "http",
 				Config: map[string]interface{}{
 					"protocol": "http",
 				},
@@ -541,6 +542,7 @@ func setupTestVariationDiscoveryChain(
 				Kind:           structs.ProxyDefaults,
 				Name:           structs.ProxyConfigGlobal,
 				EnterpriseMeta: entMeta,
+				Protocol:       "http",
 				Config: map[string]interface{}{
 					"protocol": "http",
 				},
@@ -596,6 +598,7 @@ func setupTestVariationDiscoveryChain(
 				Kind:           structs.ProxyDefaults,
 				Name:           structs.ProxyConfigGlobal,
 				EnterpriseMeta: entMeta,
+				Protocol:       "http",
 				Config: map[string]interface{}{
 					"protocol": "http",
 				},
@@ -651,6 +654,7 @@ func setupTestVariationDiscoveryChain(
 				Kind:           structs.ProxyDefaults,
 				Name:           structs.ProxyConfigGlobal,
 				EnterpriseMeta: entMeta,
+				Protocol:       "grpc",
 				Config: map[string]interface{}{
 					"protocol": "grpc",
 				},
@@ -686,6 +690,7 @@ func setupTestVariationDiscoveryChain(
 				Kind:           structs.ProxyDefaults,
 				Name:           structs.ProxyConfigGlobal,
 				EnterpriseMeta: entMeta,
+				Protocol:       "http",
 				Config: map[string]interface{}{
 					"protocol": "http",
 				},
@@ -937,6 +942,7 @@ func setupTestVariationDiscoveryChain(
 				Kind:           structs.ProxyDefaults,
 				Name:           structs.ProxyConfigGlobal,
 				EnterpriseMeta: entMeta,
+				Protocol:       "http",
 				Config: map[string]interface{}{
 					"protocol": "http",
 				},
@@ -992,6 +998,7 @@ func setupTestVariationDiscoveryChain(
 				Kind:           structs.ProxyDefaults,
 				Name:           structs.ProxyConfigGlobal,
 				EnterpriseMeta: entMeta,
+				Protocol:       "http",
 				Config: map[string]interface{}{
 					"protocol": "http",
 				},
@@ -1028,6 +1035,7 @@ func setupTestVariationDiscoveryChain(
 				Kind:           structs.ProxyDefaults,
 				Name:           structs.ProxyConfigGlobal,
 				EnterpriseMeta: entMeta,
+				Protocol:       "http",
 				Config: map[string]interface{}{
 					"protocol": "http",
 				},

diff --git a/agent/structs/config_entry.go b/agent/structs/config_entry.go
@@ -439,6 +439,7 @@ type ProxyConfigEntry struct {
 	Kind                 string
 	Name                 string
 	Config               map[string]interface{}
+	Protocol             string                               `json:"-"`
 	Mode                 ProxyMode                            `json:",omitempty"`
 	TransparentProxy     TransparentProxyConfig               `json:",omitempty" alias:"transparent_proxy"`
 	MutualTLSMode        MutualTLSMode                        `json:",omitempty" alias:"mutual_tls_mode"`

diff --git a/agent/structs/discovery_chain.go b/agent/structs/discovery_chain.go
@@ -116,7 +116,7 @@ func (s *DiscoveryGraphNode) IsResolver() bool {
 }
 
 func (s *DiscoveryGraphNode) MapKey() string {
-	return fmt.Sprintf("%s:%s", s.Type, s.Name)
+	return s.Type + ":" + s.Name
 }
 
 // compiled form of ServiceResolverConfigEntry

diff --git a/agent/xds/resources_test.go b/agent/xds/resources_test.go
@@ -688,8 +688,9 @@ func getConnectProxyDiscoChainTests(enterprise bool) []goldenTestCase {
 			create: func(t testinf.T) *proxycfg.ConfigSnapshot {
 				return proxycfg.TestConfigSnapshotDiscoveryChain(t, "simple", enterprise, nil, nil,
 					&structs.ProxyConfigEntry{
-						Kind: structs.ProxyDefaults,
-						Name: structs.ProxyConfigGlobal,
+						Kind:     structs.ProxyDefaults,
+						Name:     structs.ProxyConfigGlobal,
+						Protocol: "http",
 						Config: map[string]interface{}{
 							"protocol": "http",
 						},
@@ -703,8 +704,9 @@ func getConnectProxyDiscoChainTests(enterprise bool) []goldenTestCase {
 			create: func(t testinf.T) *proxycfg.ConfigSnapshot {
 				return proxycfg.TestConfigSnapshotDiscoveryChain(t, "simple", enterprise, nil, nil,
 					&structs.ProxyConfigEntry{
-						Kind: structs.ProxyDefaults,
-						Name: structs.ProxyConfigGlobal,
+						Kind:     structs.ProxyDefaults,
+						Name:     structs.ProxyConfigGlobal,
+						Protocol: "http2",
 						Config: map[string]interface{}{
 							"protocol": "http2",
 						},
@@ -718,8 +720,9 @@ func getConnectProxyDiscoChainTests(enterprise bool) []goldenTestCase {
 			create: func(t testinf.T) *proxycfg.ConfigSnapshot {
 				return proxycfg.TestConfigSnapshotDiscoveryChain(t, "simple", enterprise, nil, nil,
 					&structs.ProxyConfigEntry{
-						Kind: structs.ProxyDefaults,
-						Name: structs.ProxyConfigGlobal,
+						Kind:     structs.ProxyDefaults,
+						Name:     structs.ProxyConfigGlobal,
+						Protocol: "grpc",
 						Config: map[string]interface{}{
 							"protocol": "grpc",
 						},