Make several CLI commands testable

cli/cmd/get.go

@@ -5,6 +5,9 @@ import (
 	"errors"
 	"fmt"
 
+	"github.com/runconduit/conduit/cli/k8s"
+	"github.com/runconduit/conduit/cli/shell"
+
 	pb "github.com/runconduit/conduit/controller/gen/public"
 	"github.com/spf13/cobra"
 )
@@ -17,36 +20,59 @@ var getCmd = &cobra.Command{
 Valid resource types include:
  * pods (aka pod, po)`,
 	RunE: func(cmd *cobra.Command, args []string) error {
-		switch len(args) {
-		case 1:
-			resourceType := args[0]
-			switch resourceType {
-			case "pod", "pods", "po":
-				client, err := newApiClient()
-				if err != nil {
-					return err
-				}
-				resp, err := client.ListPods(context.Background(), &pb.Empty{})
-				if err != nil {
-					return err
-				}
-
-				for _, pod := range resp.GetPods() {
-					fmt.Println(pod.Name)
-				}
-
-			default:
-				return errors.New("invalid resource type")
-			}
-
-			return nil
-		default:
+		if len(args) < 1 {
 			return errors.New("please specify a resource type")
 		}
+
+		if len(args) > 1 {
+			return errors.New("please specify only one resource type")
+		}
+
+		friendlyName := args[0]
+		resourceType, err := k8s.CanonicalKubernetesNameFromFriendlyName(friendlyName)
+
+		if err != nil || resourceType != k8s.KubernetesPods {
+			return fmt.Errorf("invalid resource type %s, only %s are allowed as resource types", friendlyName, k8s.KubernetesPods)
+		}
+
+		kubeApi, err := k8s.MakeK8sAPi(shell.MakeUnixShell(), kubeconfigPath, apiAddr)
+		if err != nil {
+			return err
+		}
+
+		client, err := newApiClient(kubeApi)
+		if err != nil {
+			return err
+		}
+
+		podNames, err := getPods(client)
+		if err != nil {
+			return err
+		}
+
+		for _, podName := range podNames {
+			fmt.Println(podName)
+		}
+
+		return nil
 	},
 }
 
 func init() {
 	RootCmd.AddCommand(getCmd)
 	addControlPlaneNetworkingArgs(getCmd)
 }
+
+func getPods(apiClient pb.ApiClient) ([]string, error) {
+	resp, err := apiClient.ListPods(context.Background(), &pb.Empty{})
+	if err != nil {
+		return nil, err
+	}
+
+	names := make([]string, 0)
+	for _, pod := range resp.GetPods() {
+		names = append(names, pod.Name)
+	}
+
+	return names, nil
+}

cli/cmd/get_test.go

@@ -0,0 +1,69 @@
+package cmd
+
+import (
+	"errors"
+	"testing"
+
+	pb "github.com/runconduit/conduit/controller/gen/public"
+)
+
+func TestGetPods(t *testing.T) {
+	t.Run("Returns names of existing pods if everything went ok", func(t *testing.T) {
+		mockClient := &mockApiClient{}
+
+		pods := []*pb.Pod{
+			{Name: "pod-a"},
+			{Name: "pod-b"},
+			{Name: "pod-c"},
+		}
+
+		expectedPodNames := []string{
+			"pod-a",
+			"pod-b",
+			"pod-c",
+		}
+		response := &pb.ListPodsResponse{
+			Pods: pods,
+		}
+
+		mockClient.listPodsResponseToReturn = response
+		actualPodNames, err := getPods(mockClient)
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+
+		for i, actualName := range actualPodNames {
+			expectedName := expectedPodNames[i]
+			if expectedName != actualName {
+				t.Fatalf("Expected %dth element on %v to be [%s], but was [%s]", i, actualPodNames, expectedName, actualName)
+			}
+		}
+	})
+
+	t.Run("Returns empty list if no [ods found", func(t *testing.T) {
+		mockClient := &mockApiClient{}
+
+		mockClient.listPodsResponseToReturn = &pb.ListPodsResponse{
+			Pods: []*pb.Pod{},
+		}
+
+		actualPodNames, err := getPods(mockClient)
+		if err != nil {
+			t.Fatalf("Unexpected error: %v", err)
+		}
+
+		if len(actualPodNames) != 0 {
+			t.Fatalf("Expecting no pod names, got %v", actualPodNames)
+		}
+	})
+
+	t.Run("Returns error if cant find pods in API", func(t *testing.T) {
+		mockClient := &mockApiClient{}
+		mockClient.errorToReturn = errors.New("expected")
+
+		_, err := getPods(mockClient)
+		if err == nil {
+			t.Fatalf("Expecting error, got noting")
+		}
+	})
+}

cli/cmd/root.go

@@ -4,7 +4,6 @@ import (
 	"os"
 
 	"github.com/runconduit/conduit/cli/k8s"
-	"github.com/runconduit/conduit/cli/shell"
 	"github.com/runconduit/conduit/controller/api/public"
 	pb "github.com/runconduit/conduit/controller/gen/public"
 	"github.com/spf13/cobra"
@@ -34,12 +33,7 @@ func addControlPlaneNetworkingArgs(cmd *cobra.Command) {
 	cmd.PersistentFlags().StringVar(&apiAddr, "api-addr", "", "Override kubeconfig and communicate directly with the control plane at host:port (mostly for testing)")
 }
 
-func newApiClient() (pb.ApiClient, error) {
-	kubeApi, err := k8s.MakeK8sAPi(shell.MakeUnixShell(), kubeconfigPath, apiAddr)
-	if err != nil {
-		return nil, err
-	}
-
+func newApiClient(kubeApi k8s.KubernetesApi) (pb.ApiClient, error) {
 	url, err := kubeApi.UrlFor(controlPlaneNamespace, "/services/http:api:http/proxy/")
 	if err != nil {
 		return nil, err

cli/cmd/stat.go

@@ -9,19 +9,15 @@ import (
 	"strings"
 	"text/tabwriter"
 
+	"github.com/runconduit/conduit/cli/k8s"
+	"github.com/runconduit/conduit/cli/shell"
+
 	"github.com/runconduit/conduit/controller/api/util"
 	pb "github.com/runconduit/conduit/controller/gen/public"
 	"github.com/spf13/cobra"
 )
 
-const padding = 3
-
-type row struct {
-	requestRate float64
-	successRate float64
-	latencyP50  int64
-	latencyP99  int64
-}
+const ConduitPaths = "paths"
 
 var target string
 var timeWindow string
@@ -40,70 +36,100 @@ Valid resource types include:
 
 The optional [TARGET] option can be either a name for a deployment or pod resource`,
 	RunE: func(cmd *cobra.Command, args []string) error {
-		var resourceType string
+		var friendlyNameForResourceType string
+
 		switch len(args) {
 		case 1:
-			resourceType = args[0]
+			friendlyNameForResourceType = args[0]
 		case 2:
-			resourceType = args[0]
+			friendlyNameForResourceType = args[0]
 			target = args[1]
 		default:
 			return errors.New("please specify a resource type: pods, deployments or paths")
 		}
 
-		switch resourceType {
-		case "pods", "pod", "po":
-			return makeStatsRequest(pb.AggregationType_TARGET_POD)
-		case "deployments", "deployment", "deploy":
-			return makeStatsRequest(pb.AggregationType_TARGET_DEPLOY)
-		case "paths", "path", "pa":
-			return makeStatsRequest(pb.AggregationType_PATH)
-		default:
-			return errors.New("invalid resource type")
+		validatedResourceType, err := k8s.CanonicalKubernetesNameFromFriendlyName(friendlyNameForResourceType)
+		if err != nil {
+			switch friendlyNameForResourceType {
+			case "paths", "path", "pa":
+				validatedResourceType = ConduitPaths
+			default:
+				return fmt.Errorf("invalid resource type %s, only %v are allowed as resource types", friendlyNameForResourceType, []string{k8s.KubernetesPods, k8s.KubernetesDeployments, ConduitPaths})
+			}
+		}
+		kubeApi, err := k8s.MakeK8sAPi(shell.MakeUnixShell(), kubeconfigPath, apiAddr)
+		if err != nil {
+			return err
+		}
+
+		client, err := newApiClient(kubeApi)
+		if err != nil {
+			return fmt.Errorf("error creating api client while making stats request: %v", err)
 		}
 
-		return nil
+		output, err := requestStatsFromApi(client, validatedResourceType)
+		if err != nil {
+			return err
+		}
+
+		_, err = fmt.Print(output)
+
+		return err
 	},
 }
 
-func makeStatsRequest(aggType pb.AggregationType) error {
-	client, err := newApiClient()
-	if err != nil {
-		return fmt.Errorf("error creating api client while making stats request: %v", err)
-	}
+func init() {
+	RootCmd.AddCommand(statCmd)
+	addControlPlaneNetworkingArgs(statCmd)
+	statCmd.PersistentFlags().StringVarP(&timeWindow, "time-window", "t", "1m", "Stat window.  One of: '10s', '1m', '10m', '1h', '6h', '24h'.")
+	statCmd.PersistentFlags().BoolVarP(&watch, "watch", "w", false, "After listing/getting the requested object, watch for changes.")
+	statCmd.PersistentFlags().BoolVar(&watchOnly, "watch-only", false, "Watch for changes to the requested object(s), without listing/getting first.")
+}
+
+var resourceTypeToAggregationType = map[string]pb.AggregationType{
+	k8s.KubernetesPods:        pb.AggregationType_TARGET_POD,
+	k8s.KubernetesDeployments: pb.AggregationType_TARGET_DEPLOY,
+	ConduitPaths:              pb.AggregationType_PATH,
+}
+
+func requestStatsFromApi(client pb.ApiClient, resourceType string) (string, error) {
+	aggType := resourceTypeToAggregationType[resourceType]
 	req, err := buildMetricRequest(aggType)
 	if err != nil {
-		return fmt.Errorf("error creating metrics request while making stats request: %v", err)
+		return "", fmt.Errorf("error creating metrics request while making stats request: %v", err)
 	}
 
 	resp, err := client.Stat(context.Background(), req)
 	if err != nil {
-		return fmt.Errorf("error calling stat with request: %v", err)
+		return "", fmt.Errorf("error calling stat with request: %v", err)
 	}
 
+	return renderStats(resp)
+}
+
+func renderStats(resp *pb.MetricResponse) (string, error) {
 	var buffer bytes.Buffer
 	w := tabwriter.NewWriter(&buffer, 0, 0, padding, ' ', tabwriter.AlignRight)
-	displayStats(resp, w)
+	writeStatsToBuffer(resp, w)
 	w.Flush()
 
 	// strip left padding on the first column
 	out := string(buffer.Bytes()[padding:])
 	out = strings.Replace(out, "\n"+strings.Repeat(" ", padding), "\n", -1)
 
-	_, err = fmt.Print(out)
-	return err
+	return out, nil
 }
 
-func sortStatsKeys(stats map[string]*row) []string {
-	var sortedKeys []string
-	for key, _ := range stats {
-		sortedKeys = append(sortedKeys, key)
-	}
-	sort.Strings(sortedKeys)
-	return sortedKeys
+const padding = 3
+
+type row struct {
+	requestRate float64
+	successRate float64
+	latencyP50  int64
+	latencyP99  int64
 }
 
-func displayStats(resp *pb.MetricResponse, w *tabwriter.Writer) {
+func writeStatsToBuffer(resp *pb.MetricResponse, w *tabwriter.Writer) {
 	nameHeader := "NAME"
 	maxNameLength := len(nameHeader)
 
@@ -200,10 +226,11 @@ func buildMetricRequest(aggregationType pb.AggregationType) (*pb.MetricRequest,
 	}, nil
 }
 
-func init() {
-	RootCmd.AddCommand(statCmd)
-	addControlPlaneNetworkingArgs(statCmd)
-	statCmd.PersistentFlags().StringVarP(&timeWindow, "time-window", "t", "1m", "Stat window.  One of: '10s', '1m', '10m', '1h', '6h', '24h'.")
-	statCmd.PersistentFlags().BoolVarP(&watch, "watch", "w", false, "After listing/getting the requested object, watch for changes.")
-	statCmd.PersistentFlags().BoolVar(&watchOnly, "watch-only", false, "Watch for changes to the requested object(s), without listing/getting first.")
+func sortStatsKeys(stats map[string]*row) []string {
+	var sortedKeys []string
+	for key, _ := range stats {
+		sortedKeys = append(sortedKeys, key)
+	}
+	sort.Strings(sortedKeys)
+	return sortedKeys
 }