Opaque ports check (#6192)

Closes #6177 This change adds an additional check to the data plane category which will warn users when the `opaque-ports` annotation is misconfigured on services and pods. As an example, the output looks like this: ``` $ linkerd check --proxy ... × opaque ports are properly annotated * service/emoji-svc has the annotation config.linkerd.io/opaque-ports but pod/emoji-696d9d8f95-8p94s doesn't * pod/voting-67bb58c44c-dgt46 and service/voting-svc have the annotation config.linkerd.io/opaque-ports but values don't match see https://linkerd.io/2/checks/#linkerd-opaque-ports-definition for hints ``` Signed-off-by: Miguel Ángel Pastor Olivar <migue@github.com>
2021-06-09 23:48:43 +02:00 · 2021-06-09 23:48:43 +02:00 · 48c5f70c39
parent bc745a6bc2
commit 48c5f70c39
5 changed files with 439 additions and 3 deletions
--- a/cli/cmd/check.go
+++ b/cli/cmd/check.go
@ -186,12 +186,12 @@ func configureAndRunChecks(cmd *cobra.Command, wout io.Writer, werr io.Writer, s
 			if options.dataPlaneOnly {
 				checks = append(checks, healthcheck.LinkerdDataPlaneChecks)
 				checks = append(checks, healthcheck.LinkerdIdentityDataPlane)
+				checks = append(checks, healthcheck.LinkerdOpaquePortsDefinitionChecks)
 			} else {
 				checks = append(checks, healthcheck.LinkerdControlPlaneVersionChecks)
 			}
 			checks = append(checks, healthcheck.LinkerdCNIPluginChecks)
 			checks = append(checks, healthcheck.LinkerdHAChecks)
-
 		}
 	}

--- a/pkg/healthcheck/healthcheck.go
+++ b/pkg/healthcheck/healthcheck.go
@ -136,6 +136,11 @@ const (
 	/// plugin is installed and ready
 	LinkerdCNIPluginChecks CategoryID = "linkerd-cni-plugin"

+	// LinkerdOpaquePortsDefinitionChecks adds checks to validate that the
+	// "opaque ports" annotation has been defined both in the service and the
+	// corresponding pods
+	LinkerdOpaquePortsDefinitionChecks CategoryID = "linkerd-opaque-ports-definition"
+
 	// LinkerdCNIResourceLabel is the label key that is used to identify
 	// whether a Kubernetes resource is related to the install-cni command
 	// The value is expected to be "true", "false" or "", where "false" and
@ -1379,6 +1384,13 @@ func (hc *HealthChecker) allCategories() []*Category {
 						return checkMisconfiguredServiceAnnotations(services)
 					},
 				},
+				{
+					description: "opaque ports are properly annotated",
+					hintAnchor:  "linkerd-opaque-ports-definition",
+					check: func(ctx context.Context) error {
+						return hc.checkMisconfiguredOpaquePortAnnotations(ctx)
+					},
+				},
 			},
 			false,
 		),
@ -2192,6 +2204,87 @@ func checkResources(resourceName string, objects []runtime.Object, expectedNames
 	return nil
 }

+// Check if there's a pod with the "opaque ports" annotation defined but a
+// service selecting the aforementioned pod doesn't define it
+func (hc *HealthChecker) checkMisconfiguredOpaquePortAnnotations(ctx context.Context) error {
+	services, err := hc.GetServices(ctx)
+	if err != nil {
+		return err
+	}
+
+	var errStrings []string
+
+	for _, service := range services {
+		if service.Spec.ClusterIP == "None" {
+			// skip headless services; they're handled differently
+			continue
+		}
+
+		endpoint, err := hc.kubeAPI.CoreV1().Endpoints(service.Namespace).Get(ctx, service.Name, metav1.GetOptions{})
+
+		if err != nil {
+			return err
+		}
+
+		pods := make([]*corev1.Pod, 0)
+		for _, subset := range endpoint.Subsets {
+			for _, addr := range subset.Addresses {
+				if addr.TargetRef != nil && addr.TargetRef.Kind == "Pod" {
+					pod, err := hc.kubeAPI.CoreV1().Pods(service.Namespace).Get(ctx, addr.TargetRef.Name, metav1.GetOptions{})
+					if err != nil {
+						return err
+					}
+					pods = append(pods, pod)
+				}
+			}
+		}
+
+		if mismatch := misconfiguredOpaquePortAnnotationsInService(service, pods); mismatch != nil {
+			errStrings = append(
+				errStrings,
+				fmt.Sprintf("\t* %s", mismatch.Error()),
+			)
+		}
+	}
+
+	if len(errStrings) >= 1 {
+		return fmt.Errorf(strings.Join(errStrings, "\n    "))
+	}
+
+	return nil
+}
+
+func misconfiguredOpaquePortAnnotationsInService(service corev1.Service, pods []*corev1.Pod) error {
+	for _, pod := range pods {
+		if err := misconfiguredOpaqueAnnotation(service, pod); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func misconfiguredOpaqueAnnotation(service corev1.Service, pod *corev1.Pod) error {
+	svcAnnotation, svcAnnotationOk := service.Annotations[k8s.ProxyOpaquePortsAnnotation]
+	podAnnotation, podAnnotationOk := pod.Annotations[k8s.ProxyOpaquePortsAnnotation]
+
+	if svcAnnotationOk && podAnnotationOk {
+		if svcAnnotation != podAnnotation {
+			return fmt.Errorf("pod/%s and service/%s have the annotation %s but values don't match", pod.Name, service.Name, k8s.ProxyOpaquePortsAnnotation)
+		}
+
+		return nil
+	}
+
+	if svcAnnotationOk {
+		return fmt.Errorf("service/%s has the annotation %s but pod/%s doesn't", service.Name, k8s.ProxyOpaquePortsAnnotation, pod.Name)
+	}
+	if podAnnotationOk {
+		return fmt.Errorf("pod/%s has the annotation %s but service/%s doesn't", pod.Name, k8s.ProxyOpaquePortsAnnotation, service.Name)
+	}
+
+	return nil
+}
+
 // GetDataPlanePods returns all the pods with data plane
 func (hc *HealthChecker) GetDataPlanePods(ctx context.Context) ([]corev1.Pod, error) {
 	selector := fmt.Sprintf("%s=%s", k8s.ControllerNSLabel, hc.ControlPlaneNamespace)
--- a/pkg/healthcheck/healthcheck_test.go
+++ b/pkg/healthcheck/healthcheck_test.go
@ -1779,12 +1779,11 @@ func TestDataPlanePodLabels(t *testing.T) {
 			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())
 				}
@ -3303,6 +3302,348 @@ func TestMinReplicaCheck(t *testing.T) {
 	}
 }

+func TestCheckOpaquePortAnnotations(t *testing.T) {
+	hc := NewHealthChecker(
+		[]CategoryID{LinkerdOpaquePortsDefinitionChecks},
+		&Options{
+			DataPlaneNamespace: "test-ns",
+		},
+	)
+
+	var err error
+
+	var testCases = []struct {
+		resources []string
+		expected  error
+	}{
+		{
+			resources: []string{`
+apiVersion: v1
+kind: Service
+metadata:
+  name: test-service-1
+  namespace: test-ns
+spec:
+  ports:
+    - name: elasticsearch
+      port: 9200
+      protocol: TCP
+      targetPort: 9200
+  selector:
+    service: service-1
+`,
+				`
+apiVersion: v1
+kind: Pod
+metadata:
+  name: my-service-deployment
+  namespace: test-ns
+  labels:
+    service: service-1
+spec:
+  containers:
+    - name: test
+      image: "test-service"
+`,
+				`
+apiVersion: v1
+kind: Endpoints
+metadata:
+  annotations:
+    endpoints.kubernetes.io/last-change-trigger-time: "2021-06-08T08:38:16Z"
+  creationTimestamp: "2021-06-08T08:38:03Z"
+  labels:
+    service: test-service-1
+  name: test-service-1
+  namespace: test-ns
+subsets:
+- addresses:
+  - ip: 10.244.3.12
+    nodeName: nodename-1
+    targetRef:
+      kind: Pod
+      name: my-service-deployment
+      namespace: test-ns
+      resourceVersion: "20661"
+      uid: b37782aa-1458-4153-8399-dabc2b29aaae
+  ports:
+  - name: http-port
+    port: 8080
+    protocol: TCP
+`,
+			},
+			expected: nil,
+		},
+		{
+			resources: []string{`
+apiVersion: v1
+kind: Service
+metadata:
+  name: test-service-1
+  namespace: test-ns
+  annotations:
+    config.linkerd.io/opaque-ports: "9200"
+spec:
+  ports:
+    - name: elasticsearch
+      port: 9200
+      protocol: TCP
+      targetPort: 9200
+  selector:
+    service: service-1
+`,
+				`
+apiVersion: v1
+kind: Pod
+metadata:
+  name: my-service-deployment
+  namespace: test-ns
+  service: service-1
+  labels:
+    service: service-1
+  annotations:
+    config.linkerd.io/opaque-ports: "9200"
+spec:
+  containers:
+    - name: test
+      image: "test-service"
+`,
+				`
+apiVersion: v1
+kind: Endpoints
+metadata:
+  annotations:
+    endpoints.kubernetes.io/last-change-trigger-time: "2021-06-08T08:38:16Z"
+  creationTimestamp: "2021-06-08T08:38:03Z"
+  labels:
+    service: test-service-1
+  name: test-service-1
+  namespace: test-ns
+subsets:
+- addresses:
+  - ip: 10.244.3.12
+    nodeName: nodename-1
+    targetRef:
+      kind: Pod
+      name: my-service-deployment
+      namespace: test-ns
+      resourceVersion: "20661"
+      uid: b37782aa-1458-4153-8399-dabc2b29aaae
+  ports:
+  - name: http-port
+    port: 8080
+    protocol: TCP			
+`,
+			},
+			expected: nil,
+		},
+		{
+			resources: []string{`
+apiVersion: v1
+kind: Service
+metadata:
+  name: test-service-1
+  namespace: test-ns
+spec:
+  ports:
+    - name: http
+      port: 9200
+      protocol: TCP
+      targetPort: 9200
+  selector:
+    service: service-1
+`,
+				`
+apiVersion: v1
+kind: Pod
+metadata:
+  name: my-service-deployment
+  namespace: test-ns
+  service: service-1
+  labels:
+    service: service-1
+  annotations:
+    config.linkerd.io/opaque-ports: "9200"
+spec:
+  containers:
+    - name: test
+      image: "test-service"
+`,
+				`
+apiVersion: v1
+kind: Endpoints
+metadata:
+  annotations:
+    endpoints.kubernetes.io/last-change-trigger-time: "2021-06-08T08:38:16Z"
+  creationTimestamp: "2021-06-08T08:38:03Z"
+  labels:
+    service: test-service-1
+  name: test-service-1
+  namespace: test-ns
+subsets:
+- addresses:
+  - ip: 10.244.3.12
+    nodeName: nodename-1
+    targetRef:
+      kind: Pod
+      name: my-service-deployment
+      namespace: test-ns
+      resourceVersion: "20661"
+      uid: b37782aa-1458-4153-8399-dabc2b29aaae
+  ports:
+  - name: http-port
+    port: 8080
+    protocol: TCP
+`,
+			},
+			expected: fmt.Errorf("\t* pod/my-service-deployment has the annotation %s but service/test-service-1 doesn't", k8s.ProxyOpaquePortsAnnotation),
+		},
+		{
+			resources: []string{`
+apiVersion: v1
+kind: Service
+metadata:
+  name: test-service-1
+  namespace: test-ns
+  annotations:
+    config.linkerd.io/opaque-ports: "9200"
+spec:
+  ports:
+    - name: http
+      port: 9200
+      protocol: TCP
+      targetPort: 9200
+  selector:
+    service: service-1
+`,
+				`
+apiVersion: v1
+kind: Pod
+metadata:
+  name: my-service-deployment
+  namespace: test-ns
+  service: service-1
+  labels:
+    service: service-1
+spec:
+  containers:
+    - name: test
+      image: "test-service"
+`,
+				`
+apiVersion: v1
+kind: Endpoints
+metadata:
+  annotations:
+    endpoints.kubernetes.io/last-change-trigger-time: "2021-06-08T08:38:16Z"
+  creationTimestamp: "2021-06-08T08:38:03Z"
+  labels:
+    service: test-service-1
+  name: test-service-1
+  namespace: test-ns
+subsets:
+- addresses:
+  - ip: 10.244.3.12
+    nodeName: nodename-1
+    targetRef:
+      kind: Pod
+      name: my-service-deployment
+      namespace: test-ns
+      resourceVersion: "20661"
+      uid: b37782aa-1458-4153-8399-dabc2b29aaae
+  ports:
+  - name: http-port
+    port: 8080
+    protocol: TCP		
+`,
+			},
+			expected: fmt.Errorf("\t* service/test-service-1 has the annotation %s but pod/my-service-deployment doesn't", k8s.ProxyOpaquePortsAnnotation),
+		},
+		{
+			resources: []string{`
+apiVersion: v1
+kind: Service
+metadata:
+  name: test-service-1
+  namespace: test-ns
+  annotations:
+    config.linkerd.io/opaque-ports: "9200"
+spec:
+  ports:
+    - name: elasticsearch
+      port: 9200
+      protocol: TCP
+      targetPort: 9200
+  selector:
+    service: service-1
+`,
+				`
+apiVersion: v1
+kind: Pod
+metadata:
+  name: my-service-deployment
+  namespace: test-ns
+  service: service-1
+  labels:
+    service: service-1
+  annotations:
+    config.linkerd.io/opaque-ports: "9300"
+spec:
+  containers:
+    - name: test
+      image: "test-service"
+`,
+				`
+apiVersion: v1
+kind: Endpoints
+metadata:
+  annotations:
+    endpoints.kubernetes.io/last-change-trigger-time: "2021-06-08T08:38:16Z"
+  creationTimestamp: "2021-06-08T08:38:03Z"
+  labels:
+    service: test-service-1
+  name: test-service-1
+  namespace: test-ns
+subsets:
+- addresses:
+  - ip: 10.244.3.12
+    nodeName: nodename-1
+    targetRef:
+      kind: Pod
+      name: my-service-deployment
+      namespace: test-ns
+      resourceVersion: "20661"
+      uid: b37782aa-1458-4153-8399-dabc2b29aaae
+  ports:
+  - name: http-port
+    port: 8080
+    protocol: TCP
+`,
+			},
+			expected: fmt.Errorf("\t* pod/my-service-deployment and service/test-service-1 have the annotation %s but values don't match", k8s.ProxyOpaquePortsAnnotation),
+		},
+	}
+
+	for i, tc := range testCases {
+		tc := tc //pin
+		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
+			hc.kubeAPI, err = k8s.NewFakeAPI(tc.resources...)
+			if err != nil {
+				t.Fatalf("unexpected error: %s", err)
+			}
+			err = hc.checkMisconfiguredOpaquePortAnnotations(context.Background())
+			if err == nil && tc.expected != nil {
+				t.Fatalf("Expected check to be successful, got: %s", err)
+			}
+			if err != nil {
+				if err.Error() != tc.expected.Error() {
+					t.Fatalf("Expected error: %s, received: %s", tc.expected, err)
+				}
+			}
+		})
+	}
+}
+
 type controlPlaneReplicaOptions struct {
 	destination   int
 	identity      int
--- a/test/integration/testdata/check.cni.proxy.golden
+++ b/test/integration/testdata/check.cni.proxy.golden
@ -84,5 +84,6 @@ linkerd-data-plane
 √ data plane pod labels are configured correctly
 √ data plane service labels are configured correctly
 √ data plane service annotations are configured correctly
+√ opaque ports are properly annotated

 Status check results are √
--- a/test/integration/testdata/check.proxy.golden
+++ b/test/integration/testdata/check.proxy.golden
@ -72,5 +72,6 @@ linkerd-data-plane
 √ data plane pod labels are configured correctly
 √ data plane service labels are configured correctly
 √ data plane service annotations are configured correctly
+√ opaque ports are properly annotated

 Status check results are √