Fix issue with persisting proxy-defaults (#20481)

Fix issue with persisting proxy-defaults

This resolves an issue introduced in hashicorp/consul#19829
where the proxy-defaults configuration entry with an HTTP protocol
cannot be updated after it has been persisted once and a router
exists. This occurs because the protocol field is not properly
pre-computed before being passed into validation functions.

.changelog/20481.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+connect: Fix issue where re-persisting existing proxy-defaults using `http` protocol fails with a protocol-mismatch error.
+```

agent/config_endpoint_test.go

@@ -517,6 +517,58 @@ func TestConfig_Apply_ProxyDefaultsMeshGateway(t *testing.T) {
 	}
 }
 
+func TestConfig_Apply_ProxyDefaultsProtocol(t *testing.T) {
+	if testing.Short() {
+		t.Skip("too slow for testing.Short")
+	}
+
+	t.Parallel()
+
+	a := NewTestAgent(t, "")
+	defer a.Shutdown()
+	testrpc.WaitForTestAgent(t, a.RPC, "dc1")
+
+	writeConf := func(body string) {
+		req, _ := http.NewRequest("PUT", "/v1/config", bytes.NewBuffer([]byte(body)))
+		resp := httptest.NewRecorder()
+		_, err := a.srv.ConfigApply(resp, req)
+		require.NoError(t, err)
+		require.Equal(t, 200, resp.Code, "non-200 Response Code: %s", resp.Body.String())
+	}
+
+	// Set the default protocol
+	writeConf(`{
+		"Kind": "proxy-defaults",
+		"Name": "global",
+		"Config": {
+			"Protocol": "http"
+		}
+	}`)
+
+	// Create a router that depends on the protocol
+	writeConf(`{
+		"Kind": "service-router",
+		"Name": "route1"
+	}`)
+
+	// Ensure we can rewrite the proxy-defaults without a protocol-mismatch error.
+	// This should be taken care of in the ProxyConfigEntry.Normalize() function.
+	writeConf(`{
+		"Kind": "proxy-defaults",
+		"Name": "global",
+		"Config": {
+			"Protocol": "http",
+			"some-field": "is_changed"
+		}
+	}`)
+
+	// Rewrite the router that depends on the protocol
+	writeConf(`{
+		"Kind": "service-router",
+		"Name": "route1"
+	}`)
+}
+
 func TestConfig_Apply_CAS(t *testing.T) {
 	if testing.Short() {
 		t.Skip("too slow for testing.Short")

agent/consul/state/config_entry.go

@@ -6,7 +6,6 @@ package state
 import (
 	"errors"
 	"fmt"
-	"github.com/mitchellh/mapstructure"
 	"strings"
 
 	memdb "github.com/hashicorp/go-memdb"
@@ -528,7 +527,12 @@ func insertConfigEntryWithTxn(tx WriteTxn, idx uint64, conf structs.ConfigEntry)
 			return err
 		}
 	case structs.ProxyDefaults:
-		err := addProtocol(conf.(*structs.ProxyConfigEntry))
+		proxyDefaults, ok := conf.(*structs.ProxyConfigEntry)
+		if !ok {
+			return fmt.Errorf("unable to cast config entry to proxy-defaults")
+		}
+		// Ensure we pre-compute the protocol before persisting always.
+		err := proxyDefaults.ComputeProtocol()
 		if err != nil {
 			return err
 		}
@@ -557,21 +561,6 @@ 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

agent/structs/config_entry.go

@@ -507,6 +507,22 @@ func (e *ProxyConfigEntry) GetMeta() map[string]string {
 	return e.Meta
 }
 
+func (e *ProxyConfigEntry) ComputeProtocol() error {
+	// proxyConfig is a snippet from agent/xds/config.go:ProxyConfig
+	// We calculate this up-front so that the expensive mapstructure decode
+	// is not needed during discovery chain compile time.
+	type proxyConfig struct {
+		Protocol string `mapstructure:"protocol"`
+	}
+	var cfg proxyConfig
+	err := mapstructure.WeakDecode(e.Config, &cfg)
+	if err != nil {
+		return err
+	}
+	e.Protocol = cfg.Protocol
+	return nil
+}
+
 func (e *ProxyConfigEntry) Normalize() error {
 	if e == nil {
 		return fmt.Errorf("config entry is nil")
@@ -524,6 +540,10 @@ func (e *ProxyConfigEntry) Normalize() error {
 
 	e.EnterpriseMeta.Normalize()
 
+	if err := e.ComputeProtocol(); err != nil {
+		return err
+	}
+
 	h, err := HashConfigEntry(e)
 	if err != nil {
 		return err

agent/structs/config_entry_test.go

@@ -3662,6 +3662,47 @@ func TestProxyConfigEntry(t *testing.T) {
 	testConfigEntryNormalizeAndValidate(t, cases)
 }
 
+func TestProxyConfigEntry_ComputeProtocol(t *testing.T) {
+	t.Run("ComputeProtocol sets protocol field correctly", func(t *testing.T) {
+		pd := &ProxyConfigEntry{
+			Kind: ProxyDefaults,
+			Name: "global",
+			Config: map[string]interface{}{
+				"protocol": "http",
+			},
+		}
+		require.NoError(t, pd.ComputeProtocol())
+		require.Equal(t, &ProxyConfigEntry{
+			Kind:     ProxyDefaults,
+			Name:     "global",
+			Protocol: "http",
+			Config: map[string]interface{}{
+				"protocol": "http",
+			},
+		}, pd)
+	})
+	t.Run("Normalize sets protocol field correctly", func(t *testing.T) {
+		pd := &ProxyConfigEntry{
+			Kind: ProxyDefaults,
+			Name: "global",
+			Config: map[string]interface{}{
+				"protocol": "http",
+			},
+		}
+		require.NoError(t, pd.Normalize())
+		pd.Hash = 0
+		require.Equal(t, &ProxyConfigEntry{
+			Kind:     ProxyDefaults,
+			Name:     "global",
+			Protocol: "http",
+			Config: map[string]interface{}{
+				"protocol": "http",
+			},
+			EnterpriseMeta: *acl.DefaultEnterpriseMeta(),
+		}, pd)
+	})
+}
+
 func requireContainsLower(t *testing.T, haystack, needle string) {
 	t.Helper()
 	require.Contains(t, strings.ToLower(haystack), strings.ToLower(needle))