From 097143b273e46ebb48780710e3b621d0ed7466d9 Mon Sep 17 00:00:00 2001
From: "Chris S. Kim" <ckim@hashicorp.com>
Date: Fri, 3 Mar 2023 16:28:28 -0500
Subject: [PATCH] Add patch to preserve compatibility with pre1.10

---
 command/connect/envoy/envoy.go      | 24 ++++++++++++++++++------
 command/connect/envoy/envoy_test.go | 28 ++++++++++++++++++----------
 2 files changed, 36 insertions(+), 16 deletions(-)

diff --git a/command/connect/envoy/envoy.go b/command/connect/envoy/envoy.go
index af6ef2febf66..0864ecc19973 100644
--- a/command/connect/envoy/envoy.go
+++ b/command/connect/envoy/envoy.go
@@ -886,9 +886,12 @@ func (c *cmd) lookupXDSPort() (int, string, error) {
 
 	var resp response
 	if err := mapstructure.Decode(self, &resp); err == nil {
-		if resp.XDS.Ports.TLS <= 0 && resp.XDS.Ports.Plaintext <= 0 {
-			return 0, "", fmt.Errorf("agent has grpc disabled")
-		}
+		// When we get rid of the 1.10 compatibility code below we can uncomment
+		// this check:
+		//
+		// if resp.XDS.Ports.TLS <= 0 && resp.XDS.Ports.Plaintext <= 0 {
+		// 	return 0, "", fmt.Errorf("agent has grpc disabled")
+		// }
 		if resp.XDS.Ports.TLS > 0 {
 			return resp.XDS.Ports.TLS, "https://", nil
 		}
@@ -897,9 +900,12 @@ func (c *cmd) lookupXDSPort() (int, string, error) {
 		}
 	}
 
-	// If above TLS and Plaintext ports are both 0, fallback to
-	// old API for the case where a new consul CLI is being used
-	// with an older API version.
+	// If above TLS and Plaintext ports are both 0, it could mean
+	// gRPC is disabled on the agent or we are using an older API.
+	// In either case, fallback to reading from the DebugConfig.
+	//
+	// Next major version we should get rid of this below code.
+	// It exists for compatibility reasons for 1.10 and below.
 	cfg, ok := self["DebugConfig"]
 	if !ok {
 		return 0, "", fmt.Errorf("unexpected agent response: no debug config")
@@ -913,6 +919,12 @@ func (c *cmd) lookupXDSPort() (int, string, error) {
 		return 0, "", fmt.Errorf("invalid grpc port in agent response")
 	}
 
+	// This works for both <1.10 and later but we should prefer
+	// reading from resp.XDS instead.
+	if portN < 0 {
+		return 0, "", fmt.Errorf("agent has grpc disabled")
+	}
+
 	return int(portN), "", nil
 }
 
diff --git a/command/connect/envoy/envoy_test.go b/command/connect/envoy/envoy_test.go
index 09c02c91c094..e2692c77d7c3 100644
--- a/command/connect/envoy/envoy_test.go
+++ b/command/connect/envoy/envoy_test.go
@@ -13,7 +13,6 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/hashicorp/consul/envoyextensions/xdscommon"
 	"github.com/mitchellh/cli"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -22,6 +21,7 @@ import (
 	"github.com/hashicorp/consul/agent"
 	"github.com/hashicorp/consul/agent/xds"
 	"github.com/hashicorp/consul/api"
+	"github.com/hashicorp/consul/envoyextensions/xdscommon"
 	"github.com/hashicorp/consul/sdk/testutil"
 )
 
@@ -123,6 +123,7 @@ type generateConfigTestCase struct {
 	NamespacesEnabled bool
 	XDSPorts          agent.GRPCPorts // used to mock an agent's configured gRPC ports. Plaintext defaults to 8502 and TLS defaults to 8503.
 	AgentSelf110      bool            // fake the agent API from versions v1.10 and earlier
+	GRPCDisabled      bool
 	WantArgs          BootstrapTplArgs
 	WantErr           string
 	WantWarn          string
@@ -146,13 +147,10 @@ func TestGenerateConfig(t *testing.T) {
 			WantErr: "'-node-name' requires '-proxy-id'",
 		},
 		{
-			Name:  "gRPC disabled",
-			Flags: []string{"-proxy-id", "test-proxy"},
-			XDSPorts: agent.GRPCPorts{
-				Plaintext: -1,
-				TLS:       -1,
-			},
-			WantErr: "agent has grpc disabled",
+			Name:         "gRPC disabled",
+			Flags:        []string{"-proxy-id", "test-proxy"},
+			GRPCDisabled: true,
+			WantErr:      "agent has grpc disabled",
 		},
 		{
 			Name:  "defaults",
@@ -1387,7 +1385,7 @@ func testMockAgent(tc generateConfigTestCase) http.HandlerFunc {
 		case strings.Contains(r.URL.Path, "/agent/service"):
 			testMockAgentProxyConfig(tc.ProxyConfig, tc.NamespacesEnabled)(w, r)
 		case strings.Contains(r.URL.Path, "/agent/self"):
-			testMockAgentSelf(tc.XDSPorts, tc.AgentSelf110)(w, r)
+			testMockAgentSelf(tc.XDSPorts, tc.AgentSelf110, tc.GRPCDisabled)(w, r)
 		case strings.Contains(r.URL.Path, "/catalog/node-services"):
 			testMockCatalogNodeServiceList()(w, r)
 		case strings.Contains(r.URL.Path, "/config/proxy-defaults/global"):
@@ -1658,7 +1656,11 @@ func TestEnvoyCommand_canBindInternal(t *testing.T) {
 
 // testMockAgentSelf returns an empty /v1/agent/self response except GRPC
 // port is filled in to match the given wantXDSPort argument.
-func testMockAgentSelf(wantXDSPorts agent.GRPCPorts, agentSelf110 bool) http.HandlerFunc {
+func testMockAgentSelf(
+	wantXDSPorts agent.GRPCPorts,
+	agentSelf110 bool,
+	grpcDisabled bool,
+) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		resp := agent.Self{
 			Config: map[string]interface{}{
@@ -1670,6 +1672,12 @@ func testMockAgentSelf(wantXDSPorts agent.GRPCPorts, agentSelf110 bool) http.Han
 			resp.DebugConfig = map[string]interface{}{
 				"GRPCPort": wantXDSPorts.Plaintext,
 			}
+		} else if grpcDisabled {
+			resp.DebugConfig = map[string]interface{}{
+				"GRPCPort": -1,
+			}
+			// the real agent does not populate XDS if grpc or
+			// grpc-tls ports are < 0
 		} else {
 			resp.XDS = &agent.XDSSelf{
 				// The deprecated Port field should default to TLS if it's available.