Repository: zegl/kube-score (go)

Issue:
The statefulset-has-servicename check should check the StatefulSet SelectorLabels instead of Labels
**Which version of kube-score are you using?**

`kube-score version: 1.10.0, commit: 95faa2ad082e05d8b7637d0bd5e1ae00d1550260, built: 2020-11-07T14:17:50Z`

**What did you do?**

We've upgraded kube-score and we started seeing some errors due to `statefulset-has-servicename`. Looking at the code it looks like it expects the service selector labels to match the statefulset labels. https://github.com/zegl/kube-score/blob/ac55fb01452f9302941a5383ca7354ab711fb32c/score/apps/apps.go#L149 

This is not right as only the selectorLabels should be taken into consideration.

**What did you expect to see?**

Should use the `selectorLabels` of the StatefulSet.

**What did you see instead?** 

It uses the `labels` of the StatefulSet.



Pull request:
score/apps: fix statefulset selector labels check
Based on https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#pod-selector

This fixes #336

<!--
    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 StatefulSet selector labels check
```


Patch:
diff --git a/score/apps/apps.go b/score/apps/apps.go
index d16cc62..4721263 100644
--- a/score/apps/apps.go
+++ b/score/apps/apps.go
@@ -1,10 +1,11 @@
 package apps
 
 import (
+	"strings"
+
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"strings"
 
 	ks "github.com/zegl/kube-score/domain"
 	"github.com/zegl/kube-score/score/checks"
@@ -16,7 +17,8 @@ func Register(allChecks *checks.Checks, allHPAs []ks.HpaTargeter, allServices []
 	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 a existing headless serviceName.", statefulsetHasServiceName(allServices))
+	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)
 }
 
 func hpaDeploymentNoReplicas(allHPAs []ks.HpaTargeter) func(deployment appsv1.Deployment) (scorecard.TestScore, error) {
@@ -158,3 +160,17 @@ func statefulsetHasServiceName(allServices []ks.Service) func(statefulset appsv1
 		return
 	}
 }
+
+func statefulsetSelectorLabelsMatchTemplateMetadataLabels(statefulset appsv1.StatefulSet) (score scorecard.TestScore, err error) {
+	if internal.LabelSelectorMatchesLabels(
+		statefulset.Spec.Selector.MatchLabels,
+		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.matchLabels` to match `.spec.template.metadata.labels`. https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#pod-selector")
+	return
+}


Test patch:
diff --git a/score/apps/apps_test.go b/score/apps/apps_test.go
index 53dbe83..f2bcdf2 100644
--- a/score/apps/apps_test.go
+++ b/score/apps/apps_test.go
@@ -1,12 +1,13 @@
 package apps
 
 import (
+	"testing"
+
 	"github.com/stretchr/testify/assert"
 	appsv1 "k8s.io/api/apps/v1"
 	autoscalingv1 "k8s.io/api/autoscaling/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"testing"
 
 	ks "github.com/zegl/kube-score/domain"
 	"github.com/zegl/kube-score/scorecard"
@@ -526,7 +527,7 @@ func TestStatefulSetHasServiceName(t *testing.T) {
 			expectedSkipped: false,
 		},
 
-		// Match (multiple namespaces, revesed)
+		// Match (multiple namespaces, reversed)
 		{
 			statefulset: appsv1.StatefulSet{
 				TypeMeta:   metav1.TypeMeta{Kind: "StatefulSet"},
@@ -649,6 +650,70 @@ func TestStatefulSetHasServiceName(t *testing.T) {
 	}
 }
 
+func TestStatefulSetSelectorLabels(t *testing.T) {
+	t.Parallel()
+
+	testcases := []struct {
+		statefulset   appsv1.StatefulSet
+		expectedErr   error
+		expectedGrade scorecard.Grade
+	}{
+		// Match
+		{
+			statefulset: appsv1.StatefulSet{
+				TypeMeta:   metav1.TypeMeta{Kind: "StatefulSet"},
+				ObjectMeta: metav1.ObjectMeta{Name: "foo"},
+				Spec: appsv1.StatefulSetSpec{
+					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.StatefulSet{
+				TypeMeta:   metav1.TypeMeta{Kind: "StatefulSet"},
+				ObjectMeta: metav1.ObjectMeta{Name: "foo"},
+				Spec: appsv1.StatefulSetSpec{
+					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,
+		},
+	}
+
+	for _, tc := range testcases {
+		score, err := statefulsetSelectorLabelsMatchTemplateMetadataLabels(tc.statefulset)
+		assert.Equal(t, tc.expectedErr, err)
+		assert.Equal(t, tc.expectedGrade, score.Grade)
+	}
+}
+
 type service struct {
 	svc corev1.Service
 }
diff --git a/score/apps_test.go b/score/apps_test.go
index 27b6b5c..4560766 100644
--- a/score/apps_test.go
+++ b/score/apps_test.go
@@ -88,12 +88,17 @@ func TestStatefulsetHasServiceNameDifferentNamespace(t *testing.T) {
 	testExpectedScore(t, "statefulset-service-name-not-headless.yaml", "StatefulSet has ServiceName", scorecard.GradeCritical)
 }
 
+func TestStatefulsetHasServiceNameNotHeadless(t *testing.T) {
+	t.Parallel()
+	testExpectedScore(t, "statefulset-service-name-not-headless.yaml", "StatefulSet has ServiceName", scorecard.GradeCritical)
+}
+
 func TestStatefulsetHasServiceNameDifferentLabel(t *testing.T) {
 	t.Parallel()
 	testExpectedScore(t, "statefulset-service-name-different-label.yaml", "StatefulSet has ServiceName", scorecard.GradeCritical)
 }
 
-func TestStatefulsetHasServiceNameNotHeadless(t *testing.T) {
+func TestStatefulsetSelectorLabels(t *testing.T) {
 	t.Parallel()
-	testExpectedScore(t, "statefulset-service-name-not-headless.yaml", "StatefulSet has ServiceName", scorecard.GradeCritical)
+	testExpectedScore(t, "statefulset-different-labels.yaml", "StatefulSet Pod Selector labels match template metadata labels", scorecard.GradeCritical)
 }
diff --git a/score/security_test.go b/score/security_test.go
index df0a336..efbc1da 100644
--- a/score/security_test.go
+++ b/score/security_test.go
@@ -2,12 +2,13 @@ package score
 
 import (
 	"bytes"
+	"testing"
+
 	"github.com/stretchr/testify/assert"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/yaml"
-	"testing"
 
 	"github.com/zegl/kube-score/config"
 	ks "github.com/zegl/kube-score/domain"
@@ -192,6 +193,11 @@ func TestPodSecurityContext(test *testing.T) {
 				APIVersion: "apps/v1",
 			},
 			Spec: appsv1.StatefulSetSpec{
+				Selector: &metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"app": "foo",
+					},
+				},
 				Template: corev1.PodTemplateSpec{
 					Spec: corev1.PodSpec{
 						SecurityContext: tc.podCtx,
diff --git a/score/testdata/networkpolicy-statefulset-matching.yaml b/score/testdata/networkpolicy-statefulset-matching.yaml
index fbc0fa2..756578b 100644
--- a/score/testdata/networkpolicy-statefulset-matching.yaml
+++ b/score/testdata/networkpolicy-statefulset-matching.yaml
@@ -17,6 +17,9 @@ metadata:
   name: statefulset-test-1
   namespace: testspace
 spec:
+  selector:
+    matchLabels:
+      app: foo
   template:
     metadata:
       labels:
diff --git a/score/testdata/networkpolicy-statefulset-not-matching-selector.yaml b/score/testdata/networkpolicy-statefulset-not-matching-selector.yaml
index 673dfa6..bbd66ca 100644
--- a/score/testdata/networkpolicy-statefulset-not-matching-selector.yaml
+++ b/score/testdata/networkpolicy-statefulset-not-matching-selector.yaml
@@ -17,6 +17,9 @@ metadata:
   name: statefulset-test-1
   namespace: testspace
 spec:
+  selector:
+    matchLabels:
+      app: foo
   template:
     metadata:
       labels:
diff --git a/score/testdata/statefulset-appsv1beta1.yaml b/score/testdata/statefulset-appsv1beta1.yaml
index 00fffe1..52e818b 100644
--- a/score/testdata/statefulset-appsv1beta1.yaml
+++ b/score/testdata/statefulset-appsv1beta1.yaml
@@ -3,6 +3,9 @@ kind: StatefulSet
 metadata:
   name: statefulset-test-1
 spec:
+  selector:
+    matchLabels:
+      app: foo
   template:
     spec:
       containers:
diff --git a/score/testdata/statefulset-appsv1beta2.yaml b/score/testdata/statefulset-appsv1beta2.yaml
index 82f4060..e997c02 100644
--- a/score/testdata/statefulset-appsv1beta2.yaml
+++ b/score/testdata/statefulset-appsv1beta2.yaml
@@ -3,6 +3,9 @@ kind: StatefulSet
 metadata:
   name: statefulset-test-1
 spec:
+  selector:
+    matchLabels:
+      app: foo
   template:
     spec:
       containers:
diff --git a/score/testdata/statefulset-different-labels.yaml b/score/testdata/statefulset-different-labels.yaml
new file mode 100644
index 0000000..e7fd2da
--- /dev/null
+++ b/score/testdata/statefulset-different-labels.yaml
@@ -0,0 +1,17 @@
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: statefulset-test-1
+spec:
+  selector:
+    matchLabels:
+      app: foo
+  serviceName: svc-test-1
+  template:
+    metadata:
+      labels:
+        app: bar
+    spec:
+      containers:
+      - name: foobar
+        image: foo/bar:123
diff --git a/score/testdata/statefulset-host-antiaffinity-1-replica.yaml b/score/testdata/statefulset-host-antiaffinity-1-replica.yaml
index 70b2daf..ff2a306 100644
--- a/score/testdata/statefulset-host-antiaffinity-1-replica.yaml
+++ b/score/testdata/statefulset-host-antiaffinity-1-replica.yaml
@@ -3,6 +3,9 @@ kind: StatefulSet
 metadata:
   name: statefulset-host-antiaffinity-1-replica
 spec:
+  selector:
+    matchLabels:
+      app: foo
   replicas: 1
   template:
     metadata:
diff --git a/score/testdata/statefulset-host-antiaffinity-not-set.yaml b/score/testdata/statefulset-host-antiaffinity-not-set.yaml
index bbd44a4..792e4b0 100644
--- a/score/testdata/statefulset-host-antiaffinity-not-set.yaml
+++ b/score/testdata/statefulset-host-antiaffinity-not-set.yaml
@@ -3,6 +3,9 @@ kind: StatefulSet
 metadata:
   name: statefulset-anti-affinity-not-set
 spec:
+  selector:
+    matchLabels:
+      app: foo
   replicas: 10
   template:
     metadata:
diff --git a/score/testdata/statefulset-host-antiaffinity-preffered.yaml b/score/testdata/statefulset-host-antiaffinity-preffered.yaml
index 5aee8c4..545eb5b 100644
--- a/score/testdata/statefulset-host-antiaffinity-preffered.yaml
+++ b/score/testdata/statefulset-host-antiaffinity-preffered.yaml
@@ -3,6 +3,9 @@ kind: StatefulSet
 metadata:
   name: statefulset-preffered-anti-affinity
 spec:
+  selector:
+    matchLabels:
+      app: foo
   replicas: 10
   template:
     metadata:
@@ -16,8 +19,8 @@ spec:
             podAffinityTerm:
               labelSelector:
                 matchExpressions:
-                - key: app 
-                  operator: In 
+                - key: app
+                  operator: In
                   values:
                   - foo
               topologyKey: kubernetes.io/hostname
diff --git a/score/testdata/statefulset-host-antiaffinity-required.yaml b/score/testdata/statefulset-host-antiaffinity-required.yaml
index 96d5fee..7d301fe 100644
--- a/score/testdata/statefulset-host-antiaffinity-required.yaml
+++ b/score/testdata/statefulset-host-antiaffinity-required.yaml
@@ -3,6 +3,9 @@ kind: StatefulSet
 metadata:
   name: statefulset-required-anti-affinity
 spec:
+  selector:
+    matchLabels:
+      app: foo
   replicas: 10
   template:
     metadata:
diff --git a/score/testdata/statefulset-host-antiaffinity-undefined-replicas.yaml b/score/testdata/statefulset-host-antiaffinity-undefined-replicas.yaml
index 9679baa..0e7786a 100644
--- a/score/testdata/statefulset-host-antiaffinity-undefined-replicas.yaml
+++ b/score/testdata/statefulset-host-antiaffinity-undefined-replicas.yaml
@@ -3,6 +3,9 @@ kind: StatefulSet
 metadata:
   name: statefulset-host-antiaffinity-undefined-replicas
 spec:
+  selector:
+    matchLabels:
+      app: foo
   template:
     metadata:
       labels:
diff --git a/score/testdata/statefulset-poddisruptionbudget-v1beta1-expression-matches.yaml b/score/testdata/statefulset-poddisruptionbudget-v1beta1-expression-matches.yaml
index 67f2ba2..886a24b 100644
--- a/score/testdata/statefulset-poddisruptionbudget-v1beta1-expression-matches.yaml
+++ b/score/testdata/statefulset-poddisruptionbudget-v1beta1-expression-matches.yaml
@@ -15,6 +15,9 @@ kind: StatefulSet
 metadata:
   name: statefulset-test-1
 spec:
+  selector:
+    matchLabels:
+      app: foo
   template:
     metadata:
       labels:
@@ -22,4 +25,4 @@ spec:
     spec:
       containers:
       - name: foobar
-        image: foo:bar
\ No newline at end of file
+        image: foo:bar
diff --git a/score/testdata/statefulset-poddisruptionbudget-v1beta1-expression-no-match.yaml b/score/testdata/statefulset-poddisruptionbudget-v1beta1-expression-no-match.yaml
index 6ab42bc..7b22b66 100644
--- a/score/testdata/statefulset-poddisruptionbudget-v1beta1-expression-no-match.yaml
+++ b/score/testdata/statefulset-poddisruptionbudget-v1beta1-expression-no-match.yaml
@@ -15,6 +15,9 @@ kind: StatefulSet
 metadata:
   name: statefulset-test-1
 spec:
+  selector:
+    matchLabels:
+      app: foo
   template:
     metadata:
       labels:
@@ -22,4 +25,4 @@ spec:
     spec:
       containers:
       - name: foobar
-        image: foo:bar
\ No newline at end of file
+        image: foo:bar
diff --git a/score/testdata/statefulset-poddisruptionbudget-v1beta1-matches.yaml b/score/testdata/statefulset-poddisruptionbudget-v1beta1-matches.yaml
index ae2df22..55d3921 100644
--- a/score/testdata/statefulset-poddisruptionbudget-v1beta1-matches.yaml
+++ b/score/testdata/statefulset-poddisruptionbudget-v1beta1-matches.yaml
@@ -13,6 +13,9 @@ kind: StatefulSet
 metadata:
   name: statefulset-test-1
 spec:
+  selector:
+    matchLabels:
+      app: foo
   template:
     metadata:
       labels:
@@ -20,4 +23,4 @@ spec:
     spec:
       containers:
       - name: foobar
-        image: foo:bar
\ No newline at end of file
+        image: foo:bar
diff --git a/score/testdata/statefulset-poddisruptionbudget-v1beta1-no-match.yaml b/score/testdata/statefulset-poddisruptionbudget-v1beta1-no-match.yaml
index 8d03102..3541b20 100644
--- a/score/testdata/statefulset-poddisruptionbudget-v1beta1-no-match.yaml
+++ b/score/testdata/statefulset-poddisruptionbudget-v1beta1-no-match.yaml
@@ -13,6 +13,9 @@ kind: StatefulSet
 metadata:
   name: statefulset-test-1
 spec:
+  selector:
+    matchLabels:
+      app: foo
   template:
     metadata:
       labels:
@@ -20,4 +23,4 @@ spec:
     spec:
       containers:
       - name: foobar
-        image: foo:bar
\ No newline at end of file
+        image: foo:bar
diff --git a/score/testdata/statefulset-service-name-different-label.yaml b/score/testdata/statefulset-service-name-different-label.yaml
index 1272d8c..bf858c2 100644
--- a/score/testdata/statefulset-service-name-different-label.yaml
+++ b/score/testdata/statefulset-service-name-different-label.yaml
@@ -16,6 +16,9 @@ kind: StatefulSet
 metadata:
   name: statefulset-test-1
 spec:
+  selector:
+    matchLabels:
+      app: foo
   serviceName: svc-test-1
   template:
     metadata:
@@ -24,4 +27,4 @@ spec:
     spec:
       containers:
       - name: foobar
-        image: foo/bar:123
\ No newline at end of file
+        image: foo/bar:123
diff --git a/score/testdata/statefulset-service-name-different-name.yaml b/score/testdata/statefulset-service-name-different-name.yaml
index 5889ac1..ed6608c 100644
--- a/score/testdata/statefulset-service-name-different-name.yaml
+++ b/score/testdata/statefulset-service-name-different-name.yaml
@@ -16,6 +16,9 @@ kind: StatefulSet
 metadata:
   name: statefulset-test-1
 spec:
+  selector:
+    matchLabels:
+      app: foo
   serviceName: svc-test-1
   template:
     metadata:
@@ -24,4 +27,4 @@ spec:
     spec:
       containers:
       - name: foobar
-        image: foo/bar:123
\ No newline at end of file
+        image: foo/bar:123
diff --git a/score/testdata/statefulset-service-name-different-namespace.yaml b/score/testdata/statefulset-service-name-different-namespace.yaml
index b10758e..0d1943c 100644
--- a/score/testdata/statefulset-service-name-different-namespace.yaml
+++ b/score/testdata/statefulset-service-name-different-namespace.yaml
@@ -18,6 +18,9 @@ metadata:
   name: statefulset-test-1
   namespace: ns-test-1
 spec:
+  selector:
+    matchLabels:
+      app: foo
   serviceName: svc-test-1
   template:
     metadata:
@@ -26,4 +29,4 @@ spec:
     spec:
       containers:
       - name: foobar
-        image: foo/bar:123
\ No newline at end of file
+        image: foo/bar:123
diff --git a/score/testdata/statefulset-service-name-not-headless.yaml b/score/testdata/statefulset-service-name-not-headless.yaml
index 35612cd..164d3a3 100644
--- a/score/testdata/statefulset-service-name-not-headless.yaml
+++ b/score/testdata/statefulset-service-name-not-headless.yaml
@@ -15,6 +15,9 @@ kind: StatefulSet
 metadata:
   name: statefulset-test-1
 spec:
+  selector:
+    matchLabels:
+      app: foo
   serviceName: svc-test-1
   template:
     metadata:
@@ -23,4 +26,4 @@ spec:
     spec:
       containers:
       - name: foobar
-        image: foo/bar:123
\ No newline at end of file
+        image: foo/bar:123
diff --git a/score/testdata/statefulset-service-name.yaml b/score/testdata/statefulset-service-name.yaml
index d703672..408a3b3 100644
--- a/score/testdata/statefulset-service-name.yaml
+++ b/score/testdata/statefulset-service-name.yaml
@@ -16,6 +16,9 @@ kind: StatefulSet
 metadata:
   name: statefulset-test-1
 spec:
+  selector:
+    matchLabels:
+      app: foo
   serviceName: svc-test-1
   template:
     metadata:
@@ -24,4 +27,4 @@ spec:
     spec:
       containers:
       - name: foobar
-        image: foo/bar:123
\ No newline at end of file
+        image: foo/bar:123
diff --git a/score/testdata/statefulset-test-resources.yaml b/score/testdata/statefulset-test-resources.yaml
index 6db5c88..07d5eab 100644
--- a/score/testdata/statefulset-test-resources.yaml
+++ b/score/testdata/statefulset-test-resources.yaml
@@ -3,6 +3,9 @@ kind: StatefulSet
 metadata:
   name: statefulset-test-1
 spec:
+  selector:
+    matchLabels:
+      app: foo
   template:
     spec:
       containers: