Skip to content

Commit

Permalink
Add patch to preserve compatibility with pre1.10
Browse files Browse the repository at this point in the history
  • Loading branch information
Chris S. Kim committed Mar 6, 2023
1 parent d0adb5c commit 097143b
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 16 deletions.
24 changes: 18 additions & 6 deletions command/connect/envoy/envoy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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")
Expand All @@ -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
}

Expand Down
28 changes: 18 additions & 10 deletions command/connect/envoy/envoy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
)

Expand Down Expand Up @@ -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
Expand All @@ -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",
Expand Down Expand Up @@ -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"):
Expand Down Expand Up @@ -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{}{
Expand All @@ -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.
Expand Down

0 comments on commit 097143b

Please sign in to comment.