Add check verifying labels and annotation are not confused (#5718)

pkg/healthcheck/healthcheck.go

@@ -1353,6 +1353,45 @@ func (hc *HealthChecker) allCategories() []Category {
 						return nil
 					},
 				},
+				{
+					description: "data plane pod labels are configured correctly",
+					hintAnchor:  "l5d-data-plane-pod-labels",
+					warning:     true,
+					check: func(ctx context.Context) error {
+						pods, err := hc.GetDataPlanePods(ctx)
+						if err != nil {
+							return err
+						}
+
+						return checkMisconfiguredPodsLabels(pods)
+					},
+				},
+				{
+					description: "data plane service labels are configured correctly",
+					hintAnchor:  "l5d-data-plane-services-labels",
+					warning:     true,
+					check: func(ctx context.Context) error {
+						services, err := hc.GetServices(ctx)
+						if err != nil {
+							return err
+						}
+
+						return checkMisconfiguredServiceLabels(services)
+					},
+				},
+				{
+					description: "data plane service annotations are configured correctly",
+					hintAnchor:  "l5d-data-plane-services-annotations",
+					warning:     true,
+					check: func(ctx context.Context) error {
+						services, err := hc.GetServices(ctx)
+						if err != nil {
+							return err
+						}
+
+						return checkMisconfiguredServiceAnnotations(services)
+					},
+				},
 			},
 		},
 		{
@@ -2118,6 +2157,15 @@ func (hc *HealthChecker) GetDataPlanePods(ctx context.Context) ([]corev1.Pod, er
 	return podList.Items, nil
 }
 
+// GetServices returns all services within data plane namespace
+func (hc *HealthChecker) GetServices(ctx context.Context) ([]corev1.Service, error) {
+	svcList, err := hc.kubeAPI.CoreV1().Services(hc.DataPlaneNamespace).List(ctx, metav1.ListOptions{})
+	if err != nil {
+		return nil, err
+	}
+	return svcList.Items, nil
+}
+
 func (hc *HealthChecker) checkHAMetadataPresentOnKubeSystemNamespace(ctx context.Context) error {
 	ns, err := hc.kubeAPI.CoreV1().Namespaces().Get(ctx, "kube-system", metav1.GetOptions{})
 	if err != nil {

pkg/healthcheck/healthcheck_labels.go

@@ -0,0 +1,114 @@
+package healthcheck
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/linkerd/linkerd2/pkg/k8s"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+var (
+	validAsLabelOnly = []string{
+		k8s.DefaultExportedServiceSelector,
+	}
+	validAsAnnotationOnly = []string{
+		k8s.ProxyInjectAnnotation,
+	}
+	validAsAnnotationPrefixOnly = []string{
+		k8s.ProxyConfigAnnotationsPrefix,
+		k8s.ProxyConfigAnnotationsPrefixAlpha,
+	}
+)
+
+func checkMisconfiguredPodsLabels(pods []corev1.Pod) error {
+	var invalid []string
+
+	for _, pod := range pods {
+		invalidLabels := getMisconfiguredLabels(pod.ObjectMeta)
+		if len(invalidLabels) > 0 {
+			invalid = append(invalid,
+				fmt.Sprintf("\t* %s/%s\n\t\t%s", pod.Namespace, pod.Name, strings.Join(invalidLabels, "\n\t\t")))
+		}
+	}
+	if len(invalid) > 0 {
+		return fmt.Errorf("Some labels on data plane pods should be annotations:\n%s", strings.Join(invalid, "\n"))
+	}
+	return nil
+}
+
+func checkMisconfiguredServiceLabels(services []corev1.Service) error {
+	var invalid []string
+
+	for _, svc := range services {
+		invalidLabels := getMisconfiguredLabels(svc.ObjectMeta)
+		if len(invalidLabels) > 0 {
+			invalid = append(invalid,
+				fmt.Sprintf("\t* %s/%s\n\t\t%s", svc.Namespace, svc.Name, strings.Join(invalidLabels, "\n\t\t")))
+		}
+	}
+	if len(invalid) > 0 {
+		return fmt.Errorf("Some labels on data plane services should be annotations:\n%s", strings.Join(invalid, "\n"))
+	}
+	return nil
+}
+
+func checkMisconfiguredServiceAnnotations(services []corev1.Service) error {
+	var invalid []string
+
+	for _, svc := range services {
+		invalidAnnotations := getMisconfiguredAnnotations(svc.ObjectMeta)
+		if len(invalidAnnotations) > 0 {
+			invalid = append(invalid,
+				fmt.Sprintf("\t* %s/%s\n\t\t%s", svc.Namespace, svc.Name, strings.Join(invalidAnnotations, "\n\t\t")))
+		}
+	}
+	if len(invalid) > 0 {
+		return fmt.Errorf("Some annotations on data plane services should be labels:\n%s", strings.Join(invalid, "\n"))
+	}
+	return nil
+}
+
+func getMisconfiguredLabels(objectMeta metav1.ObjectMeta) []string {
+	var invalid []string
+
+	for label := range objectMeta.Labels {
+		if hasAnyPrefix(label, validAsAnnotationPrefixOnly) ||
+			containsString(label, validAsAnnotationOnly) {
+			invalid = append(invalid, label)
+		}
+	}
+
+	return invalid
+}
+
+func getMisconfiguredAnnotations(objectMeta metav1.ObjectMeta) []string {
+	var invalid []string
+
+	for ann := range objectMeta.Annotations {
+		if containsString(ann, validAsLabelOnly) {
+			invalid = append(invalid, ann)
+		}
+	}
+
+	return invalid
+}
+
+func hasAnyPrefix(str string, prefixes []string) bool {
+	for _, pref := range prefixes {
+		if strings.HasPrefix(str, pref) {
+			return true
+		}
+	}
+	return false
+}
+
+func containsString(str string, collection []string) bool {
+	for _, e := range collection {
+		if str == e {
+			return true
+		}
+	}
+	return false
+}

pkg/healthcheck/healthcheck_test.go

@@ -1943,6 +1943,202 @@ func TestValidateDataPlanePods(t *testing.T) {
 		}
 	})
 }
+
+func TestDataPlanePodLabels(t *testing.T) {
+
+	t.Run("Returns nil if pod labels are ok", func(t *testing.T) {
+		pods := []corev1.Pod{
+			{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "emoji-d9c7866bb-7v74n",
+					Annotations: map[string]string{k8s.ProxyControlPortAnnotation: "3000"},
+					Labels:      map[string]string{"app": "test"},
+				},
+			},
+		}
+
+		err := checkMisconfiguredPodsLabels(pods)
+		if err != nil {
+			t.Fatalf("Unexpected error: %s", err)
+		}
+	})
+
+	t.Run("Returns error if any labels are misconfigured", func(t *testing.T) {
+		for _, tc := range []struct {
+			description      string
+			pods             []corev1.Pod
+			expectedErrorMsg string
+		}{
+			{
+				description: "config as label",
+				pods: []corev1.Pod{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:   "emoji-d9c7866bb-7v74n",
+							Labels: map[string]string{k8s.ProxyControlPortAnnotation: "3000"},
+						},
+					},
+				},
+				expectedErrorMsg: "Some labels on data plane pods should be annotations:\n\t* /emoji-d9c7866bb-7v74n\n\t\tconfig.linkerd.io/control-port",
+			},
+			{
+				description: "alpha config as label",
+				pods: []corev1.Pod{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:   "emoji-d9c7866bb-7v74n",
+							Labels: map[string]string{k8s.ProxyConfigAnnotationsPrefixAlpha + "/alpha-setting": "3000"},
+						},
+					},
+				},
+				expectedErrorMsg: "Some labels on data plane pods should be annotations:\n\t* /emoji-d9c7866bb-7v74n\n\t\tconfig.alpha.linkerd.io/alpha-setting",
+			},
+			{
+				description: "inject annotation as label",
+				pods: []corev1.Pod{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:   "emoji-d9c7866bb-7v74n",
+							Labels: map[string]string{k8s.ProxyInjectAnnotation: "enable"},
+						},
+					},
+				},
+				expectedErrorMsg: "Some labels on data plane pods should be annotations:\n\t* /emoji-d9c7866bb-7v74n\n\t\tlinkerd.io/inject",
+			},
+		} {
+			tc := tc //pin
+			t.Run(tc.description, func(t *testing.T) {
+				err := checkMisconfiguredPodsLabels(tc.pods)
+				fmt.Println(err.Error())
+
+				if err == nil {
+					t.Fatal("Expected error, got nothing")
+				}
+				fmt.Println(err.Error())
+				if err.Error() != tc.expectedErrorMsg {
+					t.Fatalf("Unexpected error message: %s", err.Error())
+				}
+			})
+		}
+	})
+}
+
+func TestServicesLabels(t *testing.T) {
+
+	t.Run("Returns nil if service labels are ok", func(t *testing.T) {
+		services := []corev1.Service{
+			{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "emoji-d9c7866bb-7v74n",
+					Annotations: map[string]string{k8s.ProxyControlPortAnnotation: "3000"},
+					Labels:      map[string]string{"app": "test", k8s.DefaultExportedServiceSelector: "true"},
+				},
+			},
+		}
+
+		err := checkMisconfiguredServiceLabels(services)
+		if err != nil {
+			t.Fatalf("Unexpected error: %s", err)
+		}
+	})
+
+	t.Run("Returns error if service labels or annotation misconfigured", func(t *testing.T) {
+		for _, tc := range []struct {
+			description      string
+			services         []corev1.Service
+			expectedErrorMsg string
+		}{
+			{
+				description: "config as label",
+				services: []corev1.Service{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:   "emoji-d9c7866bb-7v74n",
+							Labels: map[string]string{k8s.ProxyControlPortAnnotation: "3000"},
+						},
+					},
+				},
+				expectedErrorMsg: "Some labels on data plane services should be annotations:\n\t* /emoji-d9c7866bb-7v74n\n\t\tconfig.linkerd.io/control-port",
+			},
+			{
+				description: "alpha config as label",
+				services: []corev1.Service{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:   "emoji-d9c7866bb-7v74n",
+							Labels: map[string]string{k8s.ProxyConfigAnnotationsPrefixAlpha + "/alpha-setting": "3000"},
+						},
+					},
+				},
+				expectedErrorMsg: "Some labels on data plane services should be annotations:\n\t* /emoji-d9c7866bb-7v74n\n\t\tconfig.alpha.linkerd.io/alpha-setting",
+			},
+		} {
+			tc := tc //pin
+			t.Run(tc.description, func(t *testing.T) {
+				err := checkMisconfiguredServiceLabels(tc.services)
+				if err == nil {
+					t.Fatal("Expected error, got nothing")
+				}
+				if err.Error() != tc.expectedErrorMsg {
+					t.Fatalf("Unexpected error message: %s", err.Error())
+				}
+			})
+		}
+	})
+}
+
+func TestServicesAnnotations(t *testing.T) {
+
+	t.Run("Returns nil if service annotations are ok", func(t *testing.T) {
+		services := []corev1.Service{
+			{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "emoji-d9c7866bb-7v74n",
+					Annotations: map[string]string{k8s.ProxyControlPortAnnotation: "3000"},
+					Labels:      map[string]string{"app": "test", k8s.DefaultExportedServiceSelector: "true"},
+				},
+			},
+		}
+
+		err := checkMisconfiguredServiceAnnotations(services)
+		if err != nil {
+			t.Fatalf("Unexpected error: %s", err)
+		}
+	})
+
+	t.Run("Returns error if service annotations are misconfigured", func(t *testing.T) {
+		for _, tc := range []struct {
+			description      string
+			services         []corev1.Service
+			expectedErrorMsg string
+		}{
+			{
+				description: "mirror as annotations",
+				services: []corev1.Service{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:        "emoji-d9c7866bb-7v74n",
+							Annotations: map[string]string{k8s.DefaultExportedServiceSelector: "true"},
+						},
+					},
+				},
+				expectedErrorMsg: "Some annotations on data plane services should be labels:\n\t* /emoji-d9c7866bb-7v74n\n\t\tmirror.linkerd.io/exported",
+			},
+		} {
+			tc := tc //pin
+			t.Run(tc.description, func(t *testing.T) {
+				err := checkMisconfiguredServiceAnnotations(tc.services)
+				if err == nil {
+					t.Fatal("Expected error, got nothing")
+				}
+				if err.Error() != tc.expectedErrorMsg {
+					t.Fatalf("Unexpected error message: %s", err.Error())
+				}
+			})
+		}
+	})
+}
+
 func TestLinkerdPreInstallGlobalResourcesChecks(t *testing.T) {
 	hc := NewHealthChecker(
 		[]CategoryID{LinkerdPreInstallGlobalResourcesChecks},

test/integration/testdata/check.cni.proxy.golden

@@ -78,5 +78,8 @@ linkerd-data-plane
 √ data plane proxies are ready
 √ data plane is up-to-date
 √ data plane and cli versions match
+√ data plane pod labels are configured correctly
+√ data plane service labels are configured correctly
+√ data plane service annotations are configured correctly
 
 Status check results are √

test/integration/testdata/check.proxy.golden

@@ -66,5 +66,8 @@ linkerd-data-plane
 √ data plane proxies are ready
 √ data plane is up-to-date
 √ data plane and cli versions match
+√ data plane pod labels are configured correctly
+√ data plane service labels are configured correctly
+√ data plane service annotations are configured correctly
 
 Status check results are √