Repository: zegl/kube-score (go)

Issue:
New check: Deployment Selector.MatchLabels should match the label of the Pods that it creates

Pull request:
score: add test for deployment selector matching the pod template labels
Adding support for MatchExpressions in the StatefulSet selector test as well

This fixes #382

<!--
    Optional: Add this change to the release notes by adding a RELNOTE comment
    If this shouldn't appear in the notes, simply remove this.
-->

```
RELNOTE: Add Deployment selector labels check
```


Patch:
diff --git a/README_CHECKS.md b/README_CHECKS.md
index 0e3dc7c..4aca968 100644
--- a/README_CHECKS.md
+++ b/README_CHECKS.md
@@ -14,10 +14,10 @@
 | pod-networkpolicy | Pod | Makes sure that all Pods are targeted by a NetworkPolicy | default |
 | networkpolicy-targets-pod | NetworkPolicy | Makes sure that all NetworkPolicies targets at least one Pod | default |
 | pod-probes | Pod | Makes sure that all Pods have safe probe configurations | default |
-| container-security-context | Pod | Makes sure that all pods have good securityContexts configured (*deprecated*, see [README_SECURITYCONTEXT.md](README_SECURITYCONTEXT.md) | default |
-| container-security-context-user-group-id | Pod | Makes sure that user and group ID are set and > 10000 | optional |
-| container-security-context-privileged | Pod | Makes sure that no Containers run in privileged mode | optional |
-| container-security-context-readonlyrootfilesystem | Pod | Makes sure that all Containers have a read only root filesystem | optional |
+| container-security-context | Pod | Makes sure that all pods have good securityContexts configured | default |
+| container-security-context-user-group-id | Pod | Makes sure that all pods have a security context with valid UID and GID set  | optional |
+| container-security-context-privileged | Pod | Makes sure that all pods have a unprivileged security context set | optional |
+| container-security-context-readonlyrootfilesystem | Pod | Makes sure that all pods have a security context with read only filesystem set | optional |
 | container-seccomp-profile | Pod | Makes sure that all pods have at a seccomp policy configured. | optional |
 | service-targets-pod | Service | Makes sure that all Services targets a Pod | default |
 | service-type | Service | Makes sure that the Service type is not NodePort | default |
@@ -25,6 +25,6 @@
 | deployment-has-host-podantiaffinity | Deployment | Makes sure that a podAntiAffinity has been set that prevents multiple pods from being scheduled on the same node. https://kubernetes.io/docs/concepts/configuration/assign-pod-node/ | default |
 | statefulset-has-host-podantiaffinity | StatefulSet | Makes sure that a podAntiAffinity has been set that prevents multiple pods from being scheduled on the same node. https://kubernetes.io/docs/concepts/configuration/assign-pod-node/ | default |
 | deployment-targeted-by-hpa-does-not-have-replicas-configured | Deployment | Makes sure that Deployments using a HorizontalPodAutoscaler doesn't have a statically configured replica count set | default |
-| statefulset-has-servicename | StatefulSet | Makes sure that StatefulSets have a existing serviceName that is headless. | default |
+| statefulset-has-servicename | StatefulSet | Makes sure that StatefulSets have a existing headless serviceName. | default |
 | label-values | all | Validates label values | default |
 | horizontalpodautoscaler-has-target | HorizontalPodAutoscaler | Makes sure that the HPA targets a valid object | default |
diff --git a/score/apps/apps.go b/score/apps/apps.go
index 4721263..4b907bf 100644
--- a/score/apps/apps.go
+++ b/score/apps/apps.go
@@ -1,6 +1,7 @@
 package apps
 
 import (
+	"fmt"
 	"strings"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -16,9 +17,12 @@ import (
 func Register(allChecks *checks.Checks, allHPAs []ks.HpaTargeter, allServices []ks.Service) {
 	allChecks.RegisterDeploymentCheck("Deployment has host PodAntiAffinity", "Makes sure that a podAntiAffinity has been set that prevents multiple pods from being scheduled on the same node. https://kubernetes.io/docs/concepts/configuration/assign-pod-node/", deploymentHasAntiAffinity)
 	allChecks.RegisterStatefulSetCheck("StatefulSet has host PodAntiAffinity", "Makes sure that a podAntiAffinity has been set that prevents multiple pods from being scheduled on the same node. https://kubernetes.io/docs/concepts/configuration/assign-pod-node/", statefulsetHasAntiAffinity)
+
 	allChecks.RegisterDeploymentCheck("Deployment targeted by HPA does not have replicas configured", "Makes sure that Deployments using a HorizontalPodAutoscaler doesn't have a statically configured replica count set", hpaDeploymentNoReplicas(allHPAs))
 	allChecks.RegisterStatefulSetCheck("StatefulSet has ServiceName", "Makes sure that StatefulSets have an existing headless serviceName.", statefulsetHasServiceName(allServices))
-	allChecks.RegisterStatefulSetCheck("StatefulSet Pod Selector labels match template metadata labels", "Ensure the StatefulSet selector labels match the template metadata labels.", statefulsetSelectorLabelsMatchTemplateMetadataLabels)
+
+	allChecks.RegisterDeploymentCheck("Deployment Pod Selector labels match template metadata labels", "Ensure the StatefulSet selector labels match the template metadata labels.", deploymentSelectorLabelsMatching)
+	allChecks.RegisterStatefulSetCheck("StatefulSet Pod Selector labels match template metadata labels", "Ensure the StatefulSet selector labels match the template metadata labels.", statefulSetSelectorLabelsMatching)
 }
 
 func hpaDeploymentNoReplicas(allHPAs []ks.HpaTargeter) func(deployment appsv1.Deployment) (scorecard.TestScore, error) {
@@ -161,16 +165,38 @@ func statefulsetHasServiceName(allServices []ks.Service) func(statefulset appsv1
 	}
 }
 
-func statefulsetSelectorLabelsMatchTemplateMetadataLabels(statefulset appsv1.StatefulSet) (score scorecard.TestScore, err error) {
-	if internal.LabelSelectorMatchesLabels(
-		statefulset.Spec.Selector.MatchLabels,
-		statefulset.Spec.Template.GetObjectMeta().GetLabels(),
-	) {
+func statefulSetSelectorLabelsMatching(statefulset appsv1.StatefulSet) (score scorecard.TestScore, err error) {
+	selector, err := metav1.LabelSelectorAsSelector(statefulset.Spec.Selector)
+	if err != nil {
+		score.Grade = scorecard.GradeCritical
+		score.AddComment("", "StatefulSet selector labels are not matching template metadata labels", fmt.Sprintf("Invalid selector: %s", err))
+		return
+	}
+
+	if selector.Matches(internal.MapLables(statefulset.Spec.Template.GetObjectMeta().GetLabels())) {
+		score.Grade = scorecard.GradeAllOK
+		return
+	}
+
+	score.Grade = scorecard.GradeCritical
+	score.AddComment("", "StatefulSet selector labels not matching template metadata labels", "StatefulSets require `.spec.selector` to match `.spec.template.metadata.labels`. https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#pod-selector")
+	return
+}
+
+func deploymentSelectorLabelsMatching(deployment appsv1.Deployment) (score scorecard.TestScore, err error) {
+	selector, err := metav1.LabelSelectorAsSelector(deployment.Spec.Selector)
+	if err != nil {
+		score.Grade = scorecard.GradeCritical
+		score.AddComment("", "Deployment selector labels are not matching template metadata labels", fmt.Sprintf("Invalid selector: %s", err))
+		return
+	}
+
+	if selector.Matches(internal.MapLables(deployment.Spec.Template.GetObjectMeta().GetLabels())) {
 		score.Grade = scorecard.GradeAllOK
 		return
 	}
 
 	score.Grade = scorecard.GradeCritical
-	score.AddComment("", "StatefulSet selector labels not matching template metadata labels", "StatefulSets require `.spec.selector.matchLabels` to match `.spec.template.metadata.labels`. https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#pod-selector")
+	score.AddComment("", "Deployment selector labels not matching template metadata labels", "Deployment require `.spec.selector` to match `.spec.template.metadata.labels`. https://kubernetes.io/docs/concepts/workloads/controllers/deployment/")
 	return
 }
diff --git a/score/internal/labelselector.go b/score/internal/labelselector.go
index 19243a2..ed825b3 100644
--- a/score/internal/labelselector.go
+++ b/score/internal/labelselector.go
@@ -2,7 +2,7 @@ package internal
 
 import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
-// Implements the Kubernetes Labels interface
+// MapLables implements the Kubernetes Labels interface
 type MapLables map[string]string
 
 func (l MapLables) Has(key string) bool {


Test patch:
diff --git a/score/apps/apps_test.go b/score/apps/apps_test.go
index f2bcdf2..9a5e1c3 100644
--- a/score/apps/apps_test.go
+++ b/score/apps/apps_test.go
@@ -705,10 +705,186 @@ func TestStatefulSetSelectorLabels(t *testing.T) {
 			expectedErr:   nil,
 			expectedGrade: scorecard.GradeCritical,
 		},
+
+		// Match (expression)
+		{
+			statefulset: appsv1.StatefulSet{
+				TypeMeta:   metav1.TypeMeta{Kind: "StatefulSet"},
+				ObjectMeta: metav1.ObjectMeta{Name: "foo"},
+				Spec: appsv1.StatefulSetSpec{
+					Selector: &metav1.LabelSelector{
+						MatchExpressions: []metav1.LabelSelectorRequirement{
+							{
+								Key:      "app",
+								Operator: metav1.LabelSelectorOpIn,
+								Values:   []string{"aaa", "bbb", "bar"},
+							},
+						},
+					},
+					Template: corev1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{
+							Labels: map[string]string{
+								"app": "bar",
+							},
+						},
+					},
+				},
+			},
+			expectedErr:   nil,
+			expectedGrade: scorecard.GradeAllOK,
+		},
+
+		// No match (expression)
+		{
+			statefulset: appsv1.StatefulSet{
+				TypeMeta:   metav1.TypeMeta{Kind: "StatefulSet"},
+				ObjectMeta: metav1.ObjectMeta{Name: "foo"},
+				Spec: appsv1.StatefulSetSpec{
+					Selector: &metav1.LabelSelector{
+						MatchExpressions: []metav1.LabelSelectorRequirement{
+							{
+								Key:      "app",
+								Operator: metav1.LabelSelectorOpNotIn,
+								Values:   []string{"aaa", "bbb", "bar"},
+							},
+						},
+					},
+					Template: corev1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{
+							Labels: map[string]string{
+								"app": "bar",
+							},
+						},
+					},
+				},
+			},
+			expectedErr:   nil,
+			expectedGrade: scorecard.GradeCritical,
+		},
+	}
+
+	for _, tc := range testcases {
+		score, err := statefulSetSelectorLabelsMatching(tc.statefulset)
+		assert.Equal(t, tc.expectedErr, err)
+		assert.Equal(t, tc.expectedGrade, score.Grade)
+	}
+}
+
+func TestDeploymentSelectorLabels(t *testing.T) {
+	t.Parallel()
+
+	testcases := []struct {
+		statefulset   appsv1.Deployment
+		expectedErr   error
+		expectedGrade scorecard.Grade
+	}{
+		// Match
+		{
+			statefulset: appsv1.Deployment{
+				TypeMeta:   metav1.TypeMeta{Kind: "Deployment"},
+				ObjectMeta: metav1.ObjectMeta{Name: "foo"},
+				Spec: appsv1.DeploymentSpec{
+					Selector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"app": "foo",
+						},
+					},
+					Template: corev1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{
+							Labels: map[string]string{
+								"app": "foo",
+							},
+						},
+					},
+				},
+			},
+			expectedErr:   nil,
+			expectedGrade: scorecard.GradeAllOK,
+		},
+
+		// No match (labels differ)
+		{
+			statefulset: appsv1.Deployment{
+				TypeMeta:   metav1.TypeMeta{Kind: "StatefulSet"},
+				ObjectMeta: metav1.ObjectMeta{Name: "foo"},
+				Spec: appsv1.DeploymentSpec{
+					Selector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"app": "foo",
+						},
+					},
+					Template: corev1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{
+							Labels: map[string]string{
+								"app": "bar",
+							},
+						},
+					},
+				},
+			},
+			expectedErr:   nil,
+			expectedGrade: scorecard.GradeCritical,
+		},
+
+		// Match (expression)
+		{
+			statefulset: appsv1.Deployment{
+				TypeMeta:   metav1.TypeMeta{Kind: "StatefulSet"},
+				ObjectMeta: metav1.ObjectMeta{Name: "foo"},
+				Spec: appsv1.DeploymentSpec{
+					Selector: &metav1.LabelSelector{
+						MatchExpressions: []metav1.LabelSelectorRequirement{
+							{
+								Key:      "app",
+								Operator: metav1.LabelSelectorOpIn,
+								Values:   []string{"aaa", "bbb", "bar"},
+							},
+						},
+					},
+					Template: corev1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{
+							Labels: map[string]string{
+								"app": "bar",
+							},
+						},
+					},
+				},
+			},
+			expectedErr:   nil,
+			expectedGrade: scorecard.GradeAllOK,
+		},
+
+		// No match (expression)
+		{
+			statefulset: appsv1.Deployment{
+				TypeMeta:   metav1.TypeMeta{Kind: "StatefulSet"},
+				ObjectMeta: metav1.ObjectMeta{Name: "foo"},
+				Spec: appsv1.DeploymentSpec{
+					Selector: &metav1.LabelSelector{
+						MatchExpressions: []metav1.LabelSelectorRequirement{
+							{
+								Key:      "app",
+								Operator: metav1.LabelSelectorOpNotIn,
+								Values:   []string{"aaa", "bbb", "bar"},
+							},
+						},
+					},
+					Template: corev1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{
+							Labels: map[string]string{
+								"app": "bar",
+							},
+						},
+					},
+				},
+			},
+			expectedErr:   nil,
+			expectedGrade: scorecard.GradeCritical,
+		},
 	}
 
 	for _, tc := range testcases {
-		score, err := statefulsetSelectorLabelsMatchTemplateMetadataLabels(tc.statefulset)
+		score, err := deploymentSelectorLabelsMatching(tc.statefulset)
 		assert.Equal(t, tc.expectedErr, err)
 		assert.Equal(t, tc.expectedGrade, score.Grade)
 	}