Fix some smaller Envoy Debugging bugs. 🐛

CHANGELOG.md

@@ -1,5 +1,11 @@
 ## UNRELEASED
 
+IMPROVEMENTS:
+* CLI:
+  * Display clusters by their short names rather than FQDNs for the `proxy read` command. [[GH-1412](https://github.com/hashicorp/consul-k8s/pull/1412)]
+  * Display a message when `proxy list` returns no results. [[GH-1412](https://github.com/hashicorp/consul-k8s/pull/1412)]
+  * Display a warning when a user passes a field and table filter combination to `proxy read` where the given field is not present in any of the output tables. [[GH-1412](https://github.com/hashicorp/consul-k8s/pull/1412)]
+
 ## 0.47.1 (August 12, 2022)
 
 BUG FIXES:

cli/cmd/proxy/list/command.go

@@ -21,7 +21,6 @@ type ListCommand struct {
 	*common.BaseCommand
 
 	kubernetes kubernetes.Interface
-	namespace  string
 
 	set *flag.Sets
 
@@ -129,8 +128,7 @@ func (c *ListCommand) validateFlags() error {
 	return nil
 }
 
-// initKubernetes initializes the Kubernetes client and sets the namespace based
-// on the user-provided arguments.
+// initKubernetes initializes the Kubernetes client.
 func (c *ListCommand) initKubernetes() error {
 	settings := helmCLI.New()
 
@@ -150,23 +148,27 @@ func (c *ListCommand) initKubernetes() error {
 		return fmt.Errorf("error creating Kubernetes client %v", err)
 	}
 
+	return nil
+}
+
+func (c *ListCommand) namespace() string {
+	settings := helmCLI.New()
+
 	if c.flagAllNamespaces {
-		c.namespace = "" // An empty namespace means all namespaces.
+		return "" // An empty namespace means all namespaces.
 	} else if c.flagNamespace != "" {
-		c.namespace = c.flagNamespace
+		return c.flagNamespace
 	} else {
-		c.namespace = settings.Namespace()
+		return settings.Namespace()
 	}
-
-	return err
 }
 
 // fetchPods fetches all pods in flagNamespace which run Consul proxies.
 func (c *ListCommand) fetchPods() ([]v1.Pod, error) {
 	var pods []v1.Pod
 
 	// Fetch all pods in the namespace with labels matching the gateway component names.
-	gatewaypods, err := c.kubernetes.CoreV1().Pods(c.namespace).List(c.Ctx, metav1.ListOptions{
+	gatewaypods, err := c.kubernetes.CoreV1().Pods(c.namespace()).List(c.Ctx, metav1.ListOptions{
 		LabelSelector: "component in (ingress-gateway, mesh-gateway, terminating-gateway), chart=consul-helm",
 	})
 	if err != nil {
@@ -175,7 +177,7 @@ func (c *ListCommand) fetchPods() ([]v1.Pod, error) {
 	pods = append(pods, gatewaypods.Items...)
 
 	// Fetch all pods in the namespace with a label indicating they are an API gateway.
-	apigatewaypods, err := c.kubernetes.CoreV1().Pods(c.namespace).List(c.Ctx, metav1.ListOptions{
+	apigatewaypods, err := c.kubernetes.CoreV1().Pods(c.namespace()).List(c.Ctx, metav1.ListOptions{
 		LabelSelector: "api-gateway.consul.hashicorp.com/managed=true",
 	})
 	if err != nil {
@@ -184,7 +186,7 @@ func (c *ListCommand) fetchPods() ([]v1.Pod, error) {
 	pods = append(pods, apigatewaypods.Items...)
 
 	// Fetch all pods in the namespace with a label indicating they are a service networked by Consul.
-	sidecarpods, err := c.kubernetes.CoreV1().Pods(c.namespace).List(c.Ctx, metav1.ListOptions{
+	sidecarpods, err := c.kubernetes.CoreV1().Pods(c.namespace()).List(c.Ctx, metav1.ListOptions{
 		LabelSelector: "consul.hashicorp.com/connect-inject-status=injected",
 	})
 	if err != nil {
@@ -197,10 +199,19 @@ func (c *ListCommand) fetchPods() ([]v1.Pod, error) {
 
 // output prints a table of pods to the terminal.
 func (c *ListCommand) output(pods []v1.Pod) {
+	if len(pods) == 0 {
+		if c.flagAllNamespaces {
+			c.UI.Output("No proxies found across all namespaces.")
+		} else {
+			c.UI.Output("No proxies found in %s namespace.", c.namespace())
+		}
+		return
+	}
+
 	if c.flagAllNamespaces {
-		c.UI.Output("Namespace: All Namespaces\n")
-	} else if c.namespace != "" {
-		c.UI.Output("Namespace: %s\n", c.namespace)
+		c.UI.Output("Namespace: all namespaces\n")
+	} else {
+		c.UI.Output("Namespace: %s\n", c.namespace())
 	}
 
 	var tbl *terminal.Table

cli/cmd/proxy/list/command_test.go

@@ -209,6 +209,9 @@ func TestFetchPods(t *testing.T) {
 			c := setupCommand(new(bytes.Buffer))
 			c.kubernetes = fake.NewSimpleClientset(&v1.PodList{Items: tc.pods})
 			c.flagNamespace = tc.namespace
+			if tc.namespace == "" {
+				c.flagAllNamespaces = true
+			}
 
 			pods, err := c.fetchPods()
 
@@ -308,6 +311,35 @@ func TestListCommandOutput(t *testing.T) {
 	}
 }
 
+func TestNoPodsFound(t *testing.T) {
+	cases := map[string]struct {
+		args     []string
+		expected string
+	}{
+		"Default namespace": {
+			[]string{"-n", "default"},
+			"No proxies found in default namespace.",
+		},
+		"All namespaces": {
+			[]string{"-A"},
+			"No proxies found across all namespaces.",
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			buf := new(bytes.Buffer)
+			c := setupCommand(buf)
+			c.kubernetes = fake.NewSimpleClientset()
+
+			exitCode := c.Run(tc.args)
+			require.Equal(t, 0, exitCode)
+
+			require.Contains(t, buf.String(), tc.expected)
+		})
+	}
+}
+
 func setupCommand(buf io.Writer) *ListCommand {
 	// Log at a test level to standard out.
 	log := hclog.New(&hclog.LoggerOptions{

cli/cmd/proxy/read/command.go

@@ -327,6 +327,34 @@ func (c *ReadCommand) shouldPrintTable(table bool) bool {
 	return !(c.flagClusters || c.flagEndpoints || c.flagListeners || c.flagRoutes || c.flagSecrets)
 }
 
+// filterWarnings checks if the user has passed in a combination of field and
+// table filters where the field in question is not present on the table and
+// returns a warning.
+// For example, if the user passes "-fqdn default -endpoints", a warning will
+// be printed saying "The filter `-fqdn default` does not apply to the tables displayed.".
+func (c *ReadCommand) filterWarnings() []string {
+	var warnings []string
+
+	// No table filters passed. Return early.
+	if !(c.flagClusters || c.flagEndpoints || c.flagListeners || c.flagRoutes || c.flagSecrets) {
+		return warnings
+	}
+
+	if c.flagFQDN != "" && !c.flagClusters {
+		warnings = append(warnings, fmt.Sprintf("The filter `-fqdn %s` does not apply to the tables displayed.", c.flagFQDN))
+	}
+
+	if c.flagPort != -1 && !(c.flagClusters || c.flagEndpoints || c.flagListeners) {
+		warnings = append(warnings, fmt.Sprintf("The filter `-port %d` does not apply to the tables displayed.", c.flagPort))
+	}
+
+	if c.flagAddress != "" && !(c.flagClusters || c.flagEndpoints || c.flagListeners) {
+		warnings = append(warnings, fmt.Sprintf("The filter `-address %s` does not apply to the tables displayed.", c.flagAddress))
+	}
+
+	return warnings
+}
+
 func (c *ReadCommand) outputTables(configs map[string]*EnvoyConfig) error {
 	if c.flagFQDN != "" || c.flagAddress != "" || c.flagPort != -1 {
 		c.UI.Output("Filters applied", terminal.WithHeaderStyle())
@@ -341,6 +369,10 @@ func (c *ReadCommand) outputTables(configs map[string]*EnvoyConfig) error {
 			c.UI.Output(fmt.Sprintf("Endpoint addresses with port number: %d", c.flagPort), terminal.WithInfoStyle())
 		}
 
+		for _, warning := range c.filterWarnings() {
+			c.UI.Output(warning, terminal.WithWarningStyle())
+		}
+
 		c.UI.Output("")
 	}
 

cli/cmd/proxy/read/command_test.go

@@ -61,44 +61,38 @@ func TestReadCommandOutput(t *testing.T) {
 	// These regular expressions must be present in the output.
 	expectedHeader := fmt.Sprintf("Envoy configuration for %s in namespace default:", podName)
 	expected := map[string][]string{
-		"-clusters": {"==> Clusters \\(6\\)",
+		"-clusters": {"==> Clusters \\(5\\)",
 			"Name.*FQDN.*Endpoints.*Type.*Last Updated",
-			"local_agent.*local_agent.*192\\.168\\.79\\.187:8502.*STATIC.*2022-05-13T04:22:39\\.553Z",
-			"local_app.*local_app.*127\\.0\\.0\\.1:8080.*STATIC.*2022-05-13T04:22:39\\.655Z",
-			"client.*client\\.default\\.dc1\\.internal\\.bc3815c2-1a0f-f3ff-a2e9-20d791f08d00\\.consul.*EDS.*2022-06-09T00:39:12\\.948Z",
-			"frontend.*frontend\\.default\\.dc1\\.internal\\.bc3815c2-1a0f-f3ff-a2e9-20d791f08d00\\.consul.*EDS.*2022-06-09T00:39:12\\.855Z",
-			"original-destination.*original-destination.*ORIGINAL_DST.*2022-05-13T04:22:39.743Z",
-			"server.*server.default.dc1.internal.bc3815c2-1a0f-f3ff-a2e9-20d791f08d00.consul.*EDS.*2022-06-09T00:39:12\\.754Z"},
-
-		"-endpoints": {"==> Endpoints \\(9\\)",
+			"local_agent.*192\\.168\\.79\\.187:8502.*STATIC.*2022-05-13T04:22:39\\.553Z",
+			"local_app.*127\\.0\\.0\\.1:8080.*STATIC.*2022-05-13T04:22:39\\.655Z",
+			"client.*client\\.default\\.dc1\\.internal\\.bc3815c2-1a0f-f3ff-a2e9-20d791f08d00\\.consul.*EDS",
+			"frontend.*frontend\\.default\\.dc1\\.internal\\.bc3815c2-1a0f-f3ff-a2e9-20d791f08d00\\.consul",
+			"original-destination.*ORIGINAL_DST"},
+
+		"-endpoints": {"==> Endpoints \\(6\\)",
 			"Address:Port.*Cluster.*Weight.*Status",
 			"192.168.79.187:8502.*local_agent.*1.00.*HEALTHY",
 			"127.0.0.1:8080.*local_app.*1.00.*HEALTHY",
-			"192.168.31.201:20000.*1.00.*HEALTHY",
-			"192.168.47.235:20000.*1.00.*HEALTHY",
-			"192.168.71.254:20000.*1.00.*HEALTHY",
-			"192.168.63.120:20000.*1.00.*HEALTHY",
-			"192.168.18.110:20000.*1.00.*HEALTHY",
-			"192.168.52.101:20000.*1.00.*HEALTHY",
-			"192.168.65.131:20000.*1.00.*HEALTHY"},
+			"192.168.18.110:20000.*client.*1.00.*HEALTHY",
+			"192.168.52.101:20000.*client.*1.00.*HEALTHY",
+			"192.168.65.131:20000.*client.*1.00.*HEALTHY",
+			"192.168.63.120:20000.*frontend.*1.00.*HEALTHY"},
 
 		"-listeners": {"==> Listeners \\(2\\)",
 			"Name.*Address:Port.*Direction.*Filter Chain Match.*Filters.*Last Updated",
-			"public_listener.*192\\.168\\.69\\.179:20000.*INBOUND.*Any.*\\* to local_app/.*2022-06-09T00:39:27\\.668Z",
-			"outbound_listener.*127.0.0.1:15001.*OUTBOUND.*10\\.100\\.134\\.173/32, 240\\.0\\.0\\.3/32.*to client.default.dc1.internal.bc3815c2-1a0f-f3ff-a2e9-20d791f08d00.consul.*2022-05-24T17:41:59\\.079Z",
-			"10\\.100\\.254\\.176/32, 240\\.0\\.0\\.4/32.*\\* to server\\.default\\.dc1\\.internal\\.bc3815c2-1a0f-f3ff-a2e9-20d791f08d00\\.consul/",
-			"10\\.100\\.31\\.2/32, 240\\.0\\.0\\.2/32.*to frontend\\.default\\.dc1\\.internal\\.bc3815c2-1a0f-f3ff-a2e9-20d791f08d00\\.consul",
+			"public_listener.*192\\.168\\.69\\.179:20000.*INBOUND.*Any.*\\* to local_app/",
+			"outbound_listener.*127.0.0.1:15001.*OUTBOUND.*10\\.100\\.134\\.173/32, 240\\.0\\.0\\.3/32.*to client.default.dc1.internal.bc3815c2-1a0f-f3ff-a2e9-20d791f08d00.consul",
+			"10\\.100\\.31\\.2/32, 240\\.0\\.0\\.5/32.*to frontend\\.default\\.dc1\\.internal\\.bc3815c2-1a0f-f3ff-a2e9-20d791f08d00\\.consul",
 			"Any.*to original-destination"},
 
-		"-routes": {"==> Routes \\(2\\)",
+		"-routes": {"==> Routes \\(1\\)",
 			"Name.*Destination Cluster.*Last Updated",
-			"public_listener.*local_app/.*2022-06-09T00:39:27.667Z",
-			"server.*server\\.default\\.dc1\\.internal\\.bc3815c2-1a0f-f3ff-a2e9-20d791f08d00\\.consul/.*2022-05-24T17:41:59\\.078Z"},
+			"public_listener.*local_app/"},
 
 		"-secrets": {"==> Secrets \\(2\\)",
 			"Name.*Type.*Last Updated",
-			"default.*Dynamic Active.*2022-05-24T17:41:59.078Z",
-			"ROOTCA.*Dynamic Warming.*2022-03-15T05:14:22.868Z"},
+			"default.*Dynamic Active",
+			"ROOTCA.*Dynamic Warming"},
 	}
 
 	cases := map[string][]string{
@@ -146,6 +140,106 @@ func TestReadCommandOutput(t *testing.T) {
 	}
 }
 
+// TestFilterWarnings ensures that a warning is printed if the user applies a
+// field filter (e.g. -fqdn default) and a table filter (e.g. -secrets) where
+// the former does not affect the output of the latter.
+func TestFilterWarnings(t *testing.T) {
+	podName := "fakePod"
+	cases := map[string]struct {
+		input    []string
+		warnings []string
+	}{
+		"fully qualified domain name doesn't apply to listeners": {
+			input:    []string{"-fqdn", "default", "-listeners"},
+			warnings: []string{"The filter `-fqdn default` does not apply to the tables displayed."},
+		},
+		"fully qualified domain name doesn't apply to routes": {
+			input:    []string{"-fqdn", "default", "-routes"},
+			warnings: []string{"The filter `-fqdn default` does not apply to the tables displayed."},
+		},
+		"fully qualified domain name doesn't apply to endpoints": {
+			input:    []string{"-fqdn", "default", "-endpoints"},
+			warnings: []string{"The filter `-fqdn default` does not apply to the tables displayed."},
+		},
+		"fully qualified domain name doesn't apply to secrets": {
+			input:    []string{"-fqdn", "default", "-secrets"},
+			warnings: []string{"The filter `-fqdn default` does not apply to the tables displayed."},
+		},
+		"fully qualified domain name doesn't apply to endpoints or listeners": {
+			input:    []string{"-fqdn", "default", "-endpoints", "-listeners"},
+			warnings: []string{"The filter `-fqdn default` does not apply to the tables displayed."},
+		},
+		"fully qualified domain name doesn't apply to listeners, routes, endpoints, or secrets": {
+			input:    []string{"-fqdn", "default", "-listeners", "-routes", "-endpoints", "-secrets"},
+			warnings: []string{"The filter `-fqdn default` does not apply to the tables displayed."},
+		},
+		"port doesn't apply to routes": {
+			input:    []string{"-port", "8080", "-routes"},
+			warnings: []string{"The filter `-port 8080` does not apply to the tables displayed."},
+		},
+		"port doesn't apply to secrets": {
+			input:    []string{"-port", "8080", "-secrets"},
+			warnings: []string{"The filter `-port 8080` does not apply to the tables displayed."},
+		},
+		"port doesn't apply to secrets or routes": {
+			input:    []string{"-port", "8080", "-secrets", "-routes"},
+			warnings: []string{"The filter `-port 8080` does not apply to the tables displayed."},
+		},
+		"address does not apply to routes": {
+			input:    []string{"-address", "127.0.0.1", "-routes"},
+			warnings: []string{"The filter `-address 127.0.0.1` does not apply to the tables displayed."},
+		},
+		"address does not apply to secrets": {
+			input:    []string{"-address", "127.0.0.1", "-secrets"},
+			warnings: []string{"The filter `-address 127.0.0.1` does not apply to the tables displayed."},
+		},
+		"warn address and port": {
+			input: []string{"-address", "127.0.0.1", "-port", "8080", "-secrets"},
+			warnings: []string{
+				"The filter `-address 127.0.0.1` does not apply to the tables displayed.",
+				"The filter `-port 8080` does not apply to the tables displayed.",
+			},
+		},
+		"warn fqdn, address, and port": {
+			input: []string{"-fqdn", "default", "-address", "127.0.0.1", "-port", "8080", "-secrets"},
+			warnings: []string{
+				"The filter `-fqdn default` does not apply to the tables displayed.",
+				"The filter `-address 127.0.0.1` does not apply to the tables displayed.",
+				"The filter `-port 8080` does not apply to the tables displayed.",
+			},
+		},
+		"no warning produced (happy case)": {
+			input:    []string{"-fqdn", "default", "-clusters"},
+			warnings: []string{},
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			fakePod := v1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      podName,
+					Namespace: "default",
+				},
+			}
+
+			buf := new(bytes.Buffer)
+			c := setupCommand(buf)
+			c.kubernetes = fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{fakePod}})
+			c.fetchConfig = func(context.Context, common.PortForwarder) (*EnvoyConfig, error) {
+				return testEnvoyConfig, nil
+			}
+
+			exitCode := c.Run(append([]string{podName}, tc.input...))
+			require.Equal(t, 0, exitCode) // This shouldn't error out, just warn the user.
+
+			for _, warning := range tc.warnings {
+				require.Contains(t, buf.String(), warning)
+			}
+		})
+	}
+}
+
 func setupCommand(buf io.Writer) *ReadCommand {
 	// Log at a test level to standard out.
 	log := hclog.New(&hclog.LoggerOptions{

cli/cmd/proxy/read/config.go

@@ -257,7 +257,7 @@ func parseEndpoints(rawCfg map[string]interface{}, endpointMapping map[string]st
 
 				endpoints = append(endpoints, Endpoint{
 					Address: address,
-					Cluster: cluster,
+					Cluster: strings.Split(cluster, ".")[0],
 					Weight:  lbEndpoint.LoadBalancingWeight,
 					Status:  lbEndpoint.HealthStatus,
 				})