cli: restrict metrics command to monitored namespaces (openservicemes…

…h#1737)

Ensures that metrics can only be enabled or disabled on
namespaces that are monitored.

Updates existing code in mesh_list* to reuse code to
list all the mesh names so that a namespace can be
validated for monitoring.

Part of openservicemesh#1016

cmd/cli/mesh_list.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io"
 
+	mapset "github.com/deckarep/golang-set"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	v1 "k8s.io/api/apps/v1"
@@ -51,7 +52,7 @@ func newMeshList(out io.Writer) *cobra.Command {
 }
 
 func (l *meshListCmd) run() error {
-	list, err := l.selectMeshes()
+	list, err := getControllerDeployments(l.clientSet)
 	if err != nil {
 		return errors.Errorf("Could not list deployments %v", err)
 	}
@@ -72,11 +73,24 @@ func (l *meshListCmd) run() error {
 	return nil
 }
 
-func (l *meshListCmd) selectMeshes() (*v1.DeploymentList, error) {
-	deploymentsClient := l.clientSet.AppsV1().Deployments("") // Get deployments from all namespaces
+// getControllerDeployments returns a list of Deployments corresponding to osm-controller
+func getControllerDeployments(clientSet kubernetes.Interface) (*v1.DeploymentList, error) {
+	deploymentsClient := clientSet.AppsV1().Deployments("") // Get deployments from all namespaces
 	labelSelector := metav1.LabelSelector{MatchLabels: map[string]string{"app": constants.OSMControllerName}}
 	listOptions := metav1.ListOptions{
 		LabelSelector: labels.Set(labelSelector.MatchLabels).String(),
 	}
 	return deploymentsClient.List(context.TODO(), listOptions)
 }
+
+// getMeshNames returns a set of mesh names corresponding to meshes within the cluster
+func getMeshNames(clientSet kubernetes.Interface) mapset.Set {
+	meshList := mapset.NewSet()
+
+	deploymentList, _ := getControllerDeployments(clientSet)
+	for _, elem := range deploymentList.Items {
+		meshList.Add(elem.ObjectMeta.Labels["meshName"])
+	}
+
+	return meshList
+}

cmd/cli/mesh_list_test.go

@@ -51,7 +51,7 @@ var _ = Describe("Running the mesh list command", func() {
 			addDeployment("osm-controller-2", "testMesh2", "testNs2", true)
 			addDeployment("not-osm-controller", "", "testNs3", false)
 
-			deployments, err = listCmd.selectMeshes()
+			deployments, err = getControllerDeployments(listCmd.clientSet)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(deployments.Items).To(gstruct.MatchAllElements(idSelector, gstruct.Elements{
 				"osm-controller-1/testNs1": gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{

cmd/cli/metrics.go

@@ -3,7 +3,12 @@ package main
 import (
 	"io"
 
+	mapset "github.com/deckarep/golang-set"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
+	corev1 "k8s.io/api/core/v1"
+
+	"github.com/openservicemesh/osm/pkg/constants"
 )
 
 const metricsDescription = `
@@ -23,3 +28,22 @@ func newMetricsCmd(out io.Writer) *cobra.Command {
 
 	return cmd
 }
+
+// isMonitoredNamespace returns true if the Namepspace is correctly annotated for monitoring given a set of existing meshes
+func isMonitoredNamespace(ns corev1.Namespace, meshList mapset.Set) (bool, error) {
+	// Check if the namespace has the resource monitor annotation
+	meshName, monitored := ns.Labels[constants.OSMKubeResourceMonitorAnnotation]
+	if !monitored {
+		return false, nil
+	}
+	if meshName == "" {
+		return false, errors.Errorf("Label %q on namespace %q cannot be empty",
+			constants.OSMKubeResourceMonitorAnnotation, ns.Name)
+	}
+	if !meshList.Contains(meshName) {
+		return false, errors.Errorf("Invalid mesh name %q used with label %q on namespace %q, must be one of %v",
+			meshName, constants.OSMKubeResourceMonitorAnnotation, ns.Name, meshList.ToSlice())
+	}
+
+	return true, nil
+}

cmd/cli/metrics_disable.go

@@ -64,10 +64,21 @@ func (cmd *metricsDisableCmd) run() error {
 		ctx, cancel := context.WithCancel(context.Background())
 		defer cancel()
 
-		if _, err := cmd.clientSet.CoreV1().Namespaces().Get(ctx, ns, metav1.GetOptions{}); err != nil {
+		namespace, err := cmd.clientSet.CoreV1().Namespaces().Get(ctx, ns, metav1.GetOptions{})
+		if err != nil {
 			return errors.Errorf("Failed to retrieve namespace [%s]: %v", ns, err)
 		}
 
+		// Check if the namespace belongs to a mesh, if not return an error
+		monitored, err := isMonitoredNamespace(*namespace, getMeshNames(cmd.clientSet))
+		if err != nil {
+			return err
+		}
+		if !monitored {
+			return errors.Errorf("Namespace [%s] does not belong to a mesh, missing annotation %q",
+				ns, constants.OSMKubeResourceMonitorAnnotation)
+		}
+
 		// Patch the namespace to remove the metrics annotation.
 		patch := fmt.Sprintf(`
 {
@@ -78,7 +89,7 @@ func (cmd *metricsDisableCmd) run() error {
 	}
 }`, constants.MetricsAnnotation)
 
-		_, err := cmd.clientSet.CoreV1().Namespaces().Patch(ctx, ns, types.StrategicMergePatchType, []byte(patch), metav1.PatchOptions{}, "")
+		_, err = cmd.clientSet.CoreV1().Namespaces().Patch(ctx, ns, types.StrategicMergePatchType, []byte(patch), metav1.PatchOptions{}, "")
 		if err != nil {
 			return errors.Errorf("Failed to disable metrics in namespace [%s]: %v", ns, err)
 		}

cmd/cli/metrics_enable.go

@@ -65,13 +65,25 @@ func (cmd *metricsEnableCmd) run() error {
 	// Add metrics annotation on namepaces
 	for _, ns := range cmd.namespaces {
 		ns = strings.TrimSpace(ns)
+
 		ctx, cancel := context.WithCancel(context.Background())
 		defer cancel()
 
-		if _, err := cmd.clientSet.CoreV1().Namespaces().Get(ctx, ns, metav1.GetOptions{}); err != nil {
+		namespace, err := cmd.clientSet.CoreV1().Namespaces().Get(ctx, ns, metav1.GetOptions{})
+		if err != nil {
 			return errors.Errorf("Failed to retrieve namespace [%s]: %v", ns, err)
 		}
 
+		// Check if the namespace belongs to a mesh, if not return an error
+		monitored, err := isMonitoredNamespace(*namespace, getMeshNames(cmd.clientSet))
+		if err != nil {
+			return err
+		}
+		if !monitored {
+			return errors.Errorf("Namespace [%s] does not belong to a mesh, missing annotation %q",
+				ns, constants.OSMKubeResourceMonitorAnnotation)
+		}
+
 		// Patch the namespace with metrics annotation.
 		// osm-controller uses this annotation to automatically enable new pods for metrics scraping.
 		patch := fmt.Sprintf(`
@@ -83,7 +95,7 @@ func (cmd *metricsEnableCmd) run() error {
 	}
 }`, constants.MetricsAnnotation)
 
-		_, err := cmd.clientSet.CoreV1().Namespaces().Patch(ctx, ns, types.StrategicMergePatchType, []byte(patch), metav1.PatchOptions{}, "")
+		_, err = cmd.clientSet.CoreV1().Namespaces().Patch(ctx, ns, types.StrategicMergePatchType, []byte(patch), metav1.PatchOptions{}, "")
 		if err != nil {
 			return errors.Errorf("Failed to enable metrics in namespace [%s]: %v", ns, err)
 		}

cmd/cli/metrics_test.go

@@ -3,20 +3,28 @@ package main
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"testing"
 
+	mapset "github.com/deckarep/golang-set"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/fake"
 
 	"github.com/openservicemesh/osm/pkg/constants"
 )
 
+const (
+	testMesh = "osm"
+)
+
 func newNamespace(name string, annotations map[string]string) *corev1.Namespace {
 	ns := &corev1.Namespace{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: name,
+			Name:   name,
+			Labels: map[string]string{constants.OSMKubeResourceMonitorAnnotation: testMesh},
 		},
 	}
 
@@ -27,10 +35,32 @@ func newNamespace(name string, annotations map[string]string) *corev1.Namespace
 	return ns
 }
 
+func createFakeController(fakeClient kubernetes.Interface) error {
+	controllerNs := &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "osm-system",
+		},
+	}
+
+	if _, err := fakeClient.CoreV1().Namespaces().Create(context.TODO(), controllerNs, metav1.CreateOptions{}); err != nil {
+		return err
+	}
+
+	controllerDep := createDeployment("test-controller", testMesh, true)
+	if _, err := fakeClient.AppsV1().Deployments(controllerNs.Name).Create(context.TODO(), controllerDep, metav1.CreateOptions{}); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 func TestRun_MetricsEnable(t *testing.T) {
 	assert := assert.New(t)
 	fakeClient := fake.NewSimpleClientset()
 
+	err := createFakeController(fakeClient)
+	assert.Nil(err)
+
 	type test struct {
 		cmd           *metricsEnableCmd
 		nsAnnotations map[string]string
@@ -79,6 +109,9 @@ func TestRun_MetricsDisable(t *testing.T) {
 	assert := assert.New(t)
 	fakeClient := fake.NewSimpleClientset()
 
+	err := createFakeController(fakeClient)
+	assert.Nil(err)
+
 	type test struct {
 		cmd           *metricsDisableCmd
 		nsAnnotations map[string]string
@@ -122,3 +155,55 @@ func TestRun_MetricsDisable(t *testing.T) {
 		}
 	}
 }
+
+func TestIsMonitoredNamespace(t *testing.T) {
+	assert := assert.New(t)
+
+	meshList := mapset.NewSet(testMesh)
+
+	nsMonitored := corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:   "ns-1",
+			Labels: map[string]string{constants.OSMKubeResourceMonitorAnnotation: testMesh},
+		},
+	}
+
+	nsUnmonitored := corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "ns-3",
+		},
+	}
+
+	nsInvalidMonitorLabel := corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:   "ns-4",
+			Labels: map[string]string{constants.OSMKubeResourceMonitorAnnotation: ""},
+		},
+	}
+
+	nsWrongMeshName := corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:   "ns-2",
+			Labels: map[string]string{constants.OSMKubeResourceMonitorAnnotation: "some-mesh"},
+		},
+	}
+
+	testCases := []struct {
+		ns        corev1.Namespace
+		exists    bool
+		expectErr bool
+	}{
+		{nsMonitored, true, false},
+		{nsUnmonitored, false, false},
+		{nsInvalidMonitorLabel, false, true},
+		{nsWrongMeshName, false, true},
+	}
+
+	for _, tc := range testCases {
+		t.Run(fmt.Sprintf("Testing if %s exists is monitored", tc.ns.Name), func(t *testing.T) {
+			monitored, err := isMonitoredNamespace(tc.ns, meshList)
+			assert.Equal(monitored, tc.exists)
+			assert.Equal(err != nil, tc.expectErr)
+		})
+	}
+}