From 61324cb6d14dcfd0278aebc5e1e6ece38b8230b5 Mon Sep 17 00:00:00 2001 From: neglect-yp Date: Fri, 2 Oct 2020 23:54:40 +0900 Subject: [PATCH 1/2] score/apps: StatefulSets should have a valid headless serviceName Fixes #315 --- score/apps/apps.go | 27 +- score/apps/apps_test.go | 359 ++++++++++++++++++ score/apps_test.go | 25 ++ score/score.go | 2 +- ...tefulset-service-name-different-label.yaml | 27 ++ ...atefulset-service-name-different-name.yaml | 27 ++ ...lset-service-name-different-namespace.yaml | 29 ++ ...statefulset-service-name-not-headless.yaml | 26 ++ score/testdata/statefulset-service-name.yaml | 27 ++ 9 files changed, 547 insertions(+), 2 deletions(-) create mode 100644 score/testdata/statefulset-service-name-different-label.yaml create mode 100644 score/testdata/statefulset-service-name-different-name.yaml create mode 100644 score/testdata/statefulset-service-name-different-namespace.yaml create mode 100644 score/testdata/statefulset-service-name-not-headless.yaml create mode 100644 score/testdata/statefulset-service-name.yaml diff --git a/score/apps/apps.go b/score/apps/apps.go index 1d80b0db..d16cc626 100644 --- a/score/apps/apps.go +++ b/score/apps/apps.go @@ -12,10 +12,11 @@ import ( "github.com/zegl/kube-score/scorecard" ) -func Register(allChecks *checks.Checks, allHPAs []ks.HpaTargeter) { +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 a existing headless serviceName.", statefulsetHasServiceName(allServices)) } func hpaDeploymentNoReplicas(allHPAs []ks.HpaTargeter) func(deployment appsv1.Deployment) (scorecard.TestScore, error) { @@ -133,3 +134,27 @@ func hasPodAntiAffinity(selfLables internal.MapLables, affinity *corev1.Affinity return false } + +func statefulsetHasServiceName(allServices []ks.Service) func(statefulset appsv1.StatefulSet) (scorecard.TestScore, error) { + return func(statefulset appsv1.StatefulSet) (score scorecard.TestScore, err error) { + for _, service := range allServices { + if service.Service().Namespace != statefulset.Namespace || + service.Service().Name != statefulset.Spec.ServiceName || + service.Service().Spec.ClusterIP != "None" { + continue + } + + if internal.LabelSelectorMatchesLabels( + service.Service().Spec.Selector, + statefulset.Spec.Template.GetObjectMeta().GetLabels(), + ) { + score.Grade = scorecard.GradeAllOK + return + } + } + + score.Grade = scorecard.GradeCritical + score.AddComment("", "StatefulSet does not have a valid serviceName", "StatefulSets currently require a Headless Service to be responsible for the network identity of the Pods. You are responsible for creating this Service. https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#limitations") + return + } +} diff --git a/score/apps/apps_test.go b/score/apps/apps_test.go index ac7c2ea7..53dbe832 100644 --- a/score/apps/apps_test.go +++ b/score/apps/apps_test.go @@ -301,3 +301,362 @@ func (d hpav1) HpaTarget() autoscalingv1.CrossVersionObjectReference { func (hpav1) FileLocation() ks.FileLocation { return ks.FileLocation{} } + +func TestStatefulSetHasServiceName(t *testing.T) { + t.Parallel() + + testcases := []struct { + statefulset appsv1.StatefulSet + services []ks.Service + expectedErr error + expectedGrade scorecard.Grade + expectedSkipped bool + }{ + // Match (no namespace) + { + statefulset: appsv1.StatefulSet{ + TypeMeta: metav1.TypeMeta{Kind: "StatefulSet"}, + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: appsv1.StatefulSetSpec{ + ServiceName: "foo-svc", + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": "foo", + }, + }, + }, + }, + }, + services: []ks.Service{ + service{ + corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo-svc"}, + Spec: corev1.ServiceSpec{ + ClusterIP: "None", + Selector: map[string]string{ + "app": "foo", + }, + }, + }, + }, + }, + expectedErr: nil, + expectedGrade: scorecard.GradeAllOK, + expectedSkipped: false, + }, + + // No match (different service name) + { + statefulset: appsv1.StatefulSet{ + TypeMeta: metav1.TypeMeta{Kind: "StatefulSet"}, + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: appsv1.StatefulSetSpec{ + ServiceName: "bar-svc", + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": "foo", + }, + }, + }, + }, + }, + services: []ks.Service{ + service{ + corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo-svc"}, + Spec: corev1.ServiceSpec{ + ClusterIP: "None", + Selector: map[string]string{ + "app": "foo", + }, + }, + }, + }, + }, + expectedErr: nil, + expectedGrade: scorecard.GradeCritical, + expectedSkipped: false, + }, + + // No match (missing service name) + { + statefulset: appsv1.StatefulSet{ + TypeMeta: metav1.TypeMeta{Kind: "StatefulSet"}, + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: appsv1.StatefulSetSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": "foo", + }, + }, + }, + }, + }, + services: []ks.Service{ + service{ + corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo-svc"}, + Spec: corev1.ServiceSpec{ + ClusterIP: "None", + Selector: map[string]string{ + "app": "foo", + }, + }, + }, + }, + }, + expectedErr: nil, + expectedGrade: scorecard.GradeCritical, + expectedSkipped: false, + }, + + // Match (same namespace) + { + statefulset: appsv1.StatefulSet{ + TypeMeta: metav1.TypeMeta{Kind: "StatefulSet"}, + ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "foo-ns"}, + Spec: appsv1.StatefulSetSpec{ + ServiceName: "foo-svc", + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": "foo", + }, + }, + }, + }, + }, + services: []ks.Service{ + service{ + corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo-svc", Namespace: "foo-ns"}, + Spec: corev1.ServiceSpec{ + ClusterIP: "None", + Selector: map[string]string{ + "app": "foo", + }, + }, + }, + }, + }, + expectedErr: nil, + expectedGrade: scorecard.GradeAllOK, + expectedSkipped: false, + }, + + // No match (different namespace) + { + statefulset: appsv1.StatefulSet{ + TypeMeta: metav1.TypeMeta{Kind: "StatefulSet"}, + ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "foo-ns"}, + Spec: appsv1.StatefulSetSpec{ + ServiceName: "foo-svc", + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": "foo", + }, + }, + }, + }, + }, + services: []ks.Service{ + service{ + corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo-svc", Namespace: "bar-ns"}, + Spec: corev1.ServiceSpec{ + ClusterIP: "None", + Selector: map[string]string{ + "app": "foo", + }, + }, + }, + }, + }, + expectedErr: nil, + expectedGrade: scorecard.GradeCritical, + expectedSkipped: false, + }, + + // Match (multiple namespaces) + { + statefulset: appsv1.StatefulSet{ + TypeMeta: metav1.TypeMeta{Kind: "StatefulSet"}, + ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "foo-ns"}, + Spec: appsv1.StatefulSetSpec{ + ServiceName: "foo-svc", + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": "foo", + }, + }, + }, + }, + }, + services: []ks.Service{ + service{ + corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo-svc", Namespace: "bar-ns"}, + Spec: corev1.ServiceSpec{ + ClusterIP: "None", + Selector: map[string]string{ + "app": "foo", + }, + }, + }, + }, + service{ + corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo-svc", Namespace: "foo-ns"}, + Spec: corev1.ServiceSpec{ + ClusterIP: "None", + Selector: map[string]string{ + "app": "foo", + }, + }, + }, + }, + }, + expectedErr: nil, + expectedGrade: scorecard.GradeAllOK, + expectedSkipped: false, + }, + + // Match (multiple namespaces, revesed) + { + statefulset: appsv1.StatefulSet{ + TypeMeta: metav1.TypeMeta{Kind: "StatefulSet"}, + ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "foo-ns"}, + Spec: appsv1.StatefulSetSpec{ + ServiceName: "foo-svc", + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": "foo", + }, + }, + }, + }, + }, + services: []ks.Service{ + service{ + corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo-svc", Namespace: "foo-ns"}, + Spec: corev1.ServiceSpec{ + ClusterIP: "None", + Selector: map[string]string{ + "app": "foo", + }, + }, + }, + }, + service{ + corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo-svc", Namespace: "bar-ns"}, + Spec: corev1.ServiceSpec{ + ClusterIP: "None", + Selector: map[string]string{ + "app": "foo", + }, + }, + }, + }, + }, + expectedErr: nil, + expectedGrade: scorecard.GradeAllOK, + expectedSkipped: false, + }, + + // No match (not headless service) + { + statefulset: appsv1.StatefulSet{ + TypeMeta: metav1.TypeMeta{Kind: "StatefulSet"}, + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: appsv1.StatefulSetSpec{ + ServiceName: "foo-svc", + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": "foo", + }, + }, + }, + }, + }, + services: []ks.Service{ + service{ + corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo-svc"}, + Spec: corev1.ServiceSpec{ + ClusterIP: "", + Selector: map[string]string{ + "app": "foo", + }, + }, + }, + }, + }, + expectedErr: nil, + expectedGrade: scorecard.GradeCritical, + expectedSkipped: false, + }, + + // No match (selector) + { + statefulset: appsv1.StatefulSet{ + TypeMeta: metav1.TypeMeta{Kind: "StatefulSet"}, + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: appsv1.StatefulSetSpec{ + ServiceName: "foo-svc", + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": "foo", + }, + }, + }, + }, + }, + services: []ks.Service{ + service{ + corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "foo-svc"}, + Spec: corev1.ServiceSpec{ + ClusterIP: "None", + Selector: map[string]string{ + "app": "bar", + }, + }, + }, + }, + }, + expectedErr: nil, + expectedGrade: scorecard.GradeCritical, + expectedSkipped: false, + }, + } + + for _, tc := range testcases { + fn := statefulsetHasServiceName(tc.services) + score, err := fn(tc.statefulset) + assert.Equal(t, tc.expectedErr, err) + assert.Equal(t, tc.expectedGrade, score.Grade) + assert.Equal(t, tc.expectedSkipped, score.Skipped) + } +} + +type service struct { + svc corev1.Service +} + +func (d service) Service() corev1.Service { + return d.svc +} + +func (d service) FileLocation() ks.FileLocation { + return ks.FileLocation{} +} diff --git a/score/apps_test.go b/score/apps_test.go index e4021aea..27b6b5c9 100644 --- a/score/apps_test.go +++ b/score/apps_test.go @@ -72,3 +72,28 @@ func TestDeploymentWithHPANotHasReplicas(t *testing.T) { t.Parallel() testExpectedScore(t, "deployment-with-hpa-not-has-replicas.yaml", "Deployment targeted by HPA does not have replicas configured", scorecard.GradeAllOK) } + +func TestStatefulsetHasServiceName(t *testing.T) { + t.Parallel() + testExpectedScore(t, "statefulset-service-name.yaml", "StatefulSet has ServiceName", scorecard.GradeAllOK) +} + +func TestStatefulsetHasServiceNameDifferentName(t *testing.T) { + t.Parallel() + testExpectedScore(t, "statefulset-service-name-different-name.yaml", "StatefulSet has ServiceName", scorecard.GradeCritical) +} + +func TestStatefulsetHasServiceNameDifferentNamespace(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) { + t.Parallel() + testExpectedScore(t, "statefulset-service-name-not-headless.yaml", "StatefulSet has ServiceName", scorecard.GradeCritical) +} diff --git a/score/score.go b/score/score.go index 0b743278..007b31d3 100644 --- a/score/score.go +++ b/score/score.go @@ -34,7 +34,7 @@ func RegisterAllChecks(allObjects ks.AllTypes, cnf config.Configuration) *checks security.Register(allChecks) service.Register(allChecks, allObjects, allObjects) stable.Register(cnf.KubernetesVersion, allChecks) - apps.Register(allChecks, allObjects.HorizontalPodAutoscalers()) + apps.Register(allChecks, allObjects.HorizontalPodAutoscalers(), allObjects.Services()) meta.Register(allChecks) hpa.Register(allChecks, allObjects.Metas()) diff --git a/score/testdata/statefulset-service-name-different-label.yaml b/score/testdata/statefulset-service-name-different-label.yaml new file mode 100644 index 00000000..1272d8cc --- /dev/null +++ b/score/testdata/statefulset-service-name-different-label.yaml @@ -0,0 +1,27 @@ +apiVersion: v1 +kind: Service +metadata: + name: svc-test-1 +spec: + clusterIP: "None" + selector: + app: foo + ports: + - protocol: TCP + port: 80 + targetPort: 8080 +--- +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: statefulset-test-1 +spec: + serviceName: svc-test-1 + template: + metadata: + labels: + app: bar + spec: + containers: + - name: foobar + image: foo/bar:123 \ No newline at end of file diff --git a/score/testdata/statefulset-service-name-different-name.yaml b/score/testdata/statefulset-service-name-different-name.yaml new file mode 100644 index 00000000..5889ac10 --- /dev/null +++ b/score/testdata/statefulset-service-name-different-name.yaml @@ -0,0 +1,27 @@ +apiVersion: v1 +kind: Service +metadata: + name: svc-test-2 +spec: + clusterIP: "None" + selector: + app: foo + ports: + - protocol: TCP + port: 80 + targetPort: 8080 +--- +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: statefulset-test-1 +spec: + serviceName: svc-test-1 + template: + metadata: + labels: + app: foo + spec: + containers: + - name: foobar + image: foo/bar:123 \ No newline at end of file diff --git a/score/testdata/statefulset-service-name-different-namespace.yaml b/score/testdata/statefulset-service-name-different-namespace.yaml new file mode 100644 index 00000000..b10758e8 --- /dev/null +++ b/score/testdata/statefulset-service-name-different-namespace.yaml @@ -0,0 +1,29 @@ + +apiVersion: v1 +kind: Service +metadata: + name: svc-test-1 +spec: + clusterIP: "None" + selector: + app: foo + ports: + - protocol: TCP + port: 80 + targetPort: 8080 +--- +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: statefulset-test-1 + namespace: ns-test-1 +spec: + serviceName: svc-test-1 + template: + metadata: + labels: + app: foo + spec: + containers: + - name: foobar + image: foo/bar:123 \ No newline at end of file diff --git a/score/testdata/statefulset-service-name-not-headless.yaml b/score/testdata/statefulset-service-name-not-headless.yaml new file mode 100644 index 00000000..35612cd8 --- /dev/null +++ b/score/testdata/statefulset-service-name-not-headless.yaml @@ -0,0 +1,26 @@ +apiVersion: v1 +kind: Service +metadata: + name: svc-test-1 +spec: + selector: + app: foo + ports: + - protocol: TCP + port: 80 + targetPort: 8080 +--- +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: statefulset-test-1 +spec: + serviceName: svc-test-1 + template: + metadata: + labels: + app: foo + spec: + containers: + - name: foobar + image: foo/bar:123 \ No newline at end of file diff --git a/score/testdata/statefulset-service-name.yaml b/score/testdata/statefulset-service-name.yaml new file mode 100644 index 00000000..d7036726 --- /dev/null +++ b/score/testdata/statefulset-service-name.yaml @@ -0,0 +1,27 @@ +apiVersion: v1 +kind: Service +metadata: + name: svc-test-1 +spec: + clusterIP: "None" + selector: + app: foo + ports: + - protocol: TCP + port: 80 + targetPort: 8080 +--- +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: statefulset-test-1 +spec: + serviceName: svc-test-1 + template: + metadata: + labels: + app: foo + spec: + containers: + - name: foobar + image: foo/bar:123 \ No newline at end of file From 4d47a1d2816f10f45a2e1db24ba5eaa4f7c0beb6 Mon Sep 17 00:00:00 2001 From: neglect-yp Date: Fri, 2 Oct 2020 23:58:32 +0900 Subject: [PATCH 2/2] docs: regenerate README_CHECKS.md --- README_CHECKS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README_CHECKS.md b/README_CHECKS.md index 8db3c0f8..2f5ef731 100644 --- a/README_CHECKS.md +++ b/README_CHECKS.md @@ -22,5 +22,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 | | label-values | all | Validates label values | default | | horizontalpodautoscaler-has-target | HorizontalPodAutoscaler | Makes sure that the HPA targets a valid object | default |