Repository: zegl/kube-score (go)

Issue:
Check: Deployment targeted by HPA does not have replicas set
If a Deployment or ReplicaSet is targeted by a HorizontalPodAutoscaler, replicas should not be set in the DeploymentSpec (or ReplicaSetSpec), as applying the configuration would temporarily undo the changes made by the HPA.

Pull request:
score/apps: Deployment targeted by HPA does not have replicas configured
```
RELNOTE: New score "Deployment targeted by HPA does not have replicas configured"
```

This closes #286 


Patch:
diff --git a/README.md b/README.md
index 22dceef..927c571 100644
--- a/README.md
+++ b/README.md
@@ -2,12 +2,7 @@
 
 <p align="center"><img src="https://user-images.githubusercontent.com/47952/56085330-6c0a2480-5e41-11e9-89ba-0cfddd7714a8.png" height="200"></p>
 
-[![Go Report Card](https://goreportcard.com/badge/github.com/zegl/kube-score?)](https://goreportcard.com/report/github.com/zegl/kube-score)
-[![Build Status](https://circleci.com/gh/zegl/kube-score.svg?style=shield)](https://app.circleci.com/pipelines/github/zegl/kube-score?branch=master)
-[![Releases](https://img.shields.io/github/release-pre/zegl/kube-score.svg)](https://github.com/zegl/kube-score/releases)
-![GitHub stars](https://img.shields.io/github/stars/zegl/kube-score.svg?label=github%20stars)
-[![Downloads](https://img.shields.io/github/downloads/zegl/kube-score/total.svg)]()
-[![License](https://img.shields.io/badge/License-MIT-blue.svg)](https://github.com/zegl/kube-score/blob/master/LICENSE)
+[![Join #kube-score on the Kubernetes Slack](https://img.shields.io/badge/Slack-kubernetes%2Fkube--score-blue.svg)](https://slack.k8s.io/)
 
 ---
 
diff --git a/README_CHECKS.md b/README_CHECKS.md
index 8f6b594..379024c 100644
--- a/README_CHECKS.md
+++ b/README_CHECKS.md
@@ -21,5 +21,6 @@
 | stable-version | all | Checks if the object is using a deprecated apiVersion | default |
 | 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 | Hakes sure that Deployments using a HorizontalPodAutoscaler doesn't have a statically configured replica count set | 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 cf59078..6c9e437 100644
--- a/score/apps/apps.go
+++ b/score/apps/apps.go
@@ -2,6 +2,7 @@ package apps
 
 import (
 	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"
 
@@ -10,9 +11,36 @@ import (
 	"github.com/zegl/kube-score/scorecard"
 )
 
-func Register(allChecks *checks.Checks) {
+func Register(allChecks *checks.Checks, allHPAs []autoscalingv1.HorizontalPodAutoscaler) {
 	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", "Hakes sure that Deployments using a HorizontalPodAutoscaler doesn't have a statically configured replica count set", hpaDeploymentNoReplicas(allHPAs))
+}
+
+func hpaDeploymentNoReplicas(allHPAs []autoscalingv1.HorizontalPodAutoscaler) func(deployment appsv1.Deployment) (scorecard.TestScore, error) {
+	return func(deployment appsv1.Deployment) (score scorecard.TestScore, err error) {
+		// If is targeted by a HPA
+		for _, hpa := range allHPAs {
+			if hpa.Namespace == deployment.Namespace &&
+				hpa.Spec.ScaleTargetRef.Kind == deployment.Kind &&
+				hpa.Spec.ScaleTargetRef.Name == deployment.Name {
+
+				if deployment.Spec.Replicas == nil {
+					score.Grade = scorecard.GradeAllOK
+					return
+				}
+
+				score.Grade = scorecard.GradeCritical
+				score.AddComment("", "The deployment is targeted by a HPA, but a static replica count is configured in the DeploymentSpec", "When replicas is both statically set and managed by the HPA, the replicas will be changed to the statically configured count when the spec is applied, even if the HPA wants the replica count to be higher.")
+				return
+			}
+		}
+
+		score.Grade = scorecard.GradeAllOK
+		score.Skipped = true
+		score.AddComment("", "Skipped because the deployment is not targeted by a HorizontalPodAutoscaler", "")
+		return
+	}
 }
 
 func deploymentHasAntiAffinity(deployment appsv1.Deployment) (score scorecard.TestScore, err error) {
diff --git a/score/score.go b/score/score.go
index 64a62b4..a89dc9d 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(allChecks)
-	apps.Register(allChecks)
+	apps.Register(allChecks, allObjects.HorizontalPodAutoscalers())
 	meta.Register(allChecks)
 	hpa.Register(allChecks, allObjects.Metas())
 


Test patch:
diff --git a/score/apps/apps_test.go b/score/apps/apps_test.go
index 25c8937..f612c55 100644
--- a/score/apps/apps_test.go
+++ b/score/apps/apps_test.go
@@ -3,6 +3,7 @@ package apps
 import (
 	"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"
@@ -17,11 +18,11 @@ type testcase struct {
 	expectedSkipped bool
 }
 
-func antiAffinityTestCases() []testcase {
-	i := func(i int32) *int32 {
-		return &i
-	}
+func i(i int32) *int32 {
+	return &i
+}
 
+func antiAffinityTestCases() []testcase {
 	return []testcase{
 		{
 			// No affinity configured
@@ -151,3 +152,93 @@ func TestDeploymentHasAntiAffinity(t *testing.T) {
 		assert.Equal(t, tc.expectedSkipped, score.Skipped, "unexpected skipped, caseID=%d", caseID)
 	}
 }
+
+func TestDeploymentTargetedByHpaHasNoReplicasAllOK(t *testing.T) {
+	t.Parallel()
+
+	deployment := appsv1.Deployment{
+		TypeMeta:   metav1.TypeMeta{Kind: "Deployment"},
+		ObjectMeta: metav1.ObjectMeta{Name: "foo"},
+		Spec: appsv1.DeploymentSpec{
+			Replicas: nil,
+		},
+	}
+
+	hpas := []autoscalingv1.HorizontalPodAutoscaler{
+		{
+			Spec: autoscalingv1.HorizontalPodAutoscalerSpec{
+				ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
+					Kind:       "Deployment",
+					Name:       "foo",
+					APIVersion: "apps/v1",
+				},
+			},
+		},
+	}
+
+	f := hpaDeploymentNoReplicas(hpas)
+	score, err := f(deployment)
+	assert.Nil(t, err)
+	assert.Equal(t, scorecard.GradeAllOK, score.Grade)
+	assert.False(t, score.Skipped)
+}
+
+func TestDeploymentTargetedByHpaHasSetReplicasAllOK(t *testing.T) {
+	t.Parallel()
+
+	deployment := appsv1.Deployment{
+		TypeMeta:   metav1.TypeMeta{Kind: "Deployment"},
+		ObjectMeta: metav1.ObjectMeta{Name: "foo"},
+		Spec: appsv1.DeploymentSpec{
+			Replicas: i(30),
+		},
+	}
+
+	hpas := []autoscalingv1.HorizontalPodAutoscaler{
+		{
+			Spec: autoscalingv1.HorizontalPodAutoscalerSpec{
+				ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
+					Kind:       "Deployment",
+					Name:       "foo",
+					APIVersion: "apps/v1",
+				},
+			},
+		},
+	}
+
+	f := hpaDeploymentNoReplicas(hpas)
+	score, err := f(deployment)
+	assert.Nil(t, err)
+	assert.Equal(t, scorecard.GradeCritical, score.Grade)
+	assert.False(t, score.Skipped)
+}
+
+func TestDeploymentNotTargetedByHpaIsSkippedAllOKK(t *testing.T) {
+	t.Parallel()
+
+	deployment := appsv1.Deployment{
+		TypeMeta:   metav1.TypeMeta{Kind: "Deployment"},
+		ObjectMeta: metav1.ObjectMeta{Name: "foo"},
+		Spec: appsv1.DeploymentSpec{
+			Replicas: i(30),
+		},
+	}
+
+	hpas := []autoscalingv1.HorizontalPodAutoscaler{
+		{
+			Spec: autoscalingv1.HorizontalPodAutoscalerSpec{
+				ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
+					Kind:       "Deployment",
+					Name:       "some-other-obj",
+					APIVersion: "apps/v1",
+				},
+			},
+		},
+	}
+
+	f := hpaDeploymentNoReplicas(hpas)
+	score, err := f(deployment)
+	assert.Nil(t, err)
+	assert.Equal(t, scorecard.GradeAllOK, score.Grade)
+	assert.True(t, score.Skipped)
+}
diff --git a/score/apps_test.go b/score/apps_test.go
index 0501afa..e4021ae 100644
--- a/score/apps_test.go
+++ b/score/apps_test.go
@@ -62,3 +62,13 @@ func TestStatefulSetHasPodAntiAffinityUndefinedReplicas(t *testing.T) {
 	t.Parallel()
 	testExpectedScore(t, "statefulset-host-antiaffinity-undefined-replicas.yaml", "StatefulSet has host PodAntiAffinity", scorecard.GradeWarning)
 }
+
+func TestDeploymentWithHPAHasReplicas(t *testing.T) {
+	t.Parallel()
+	testExpectedScore(t, "deployment-with-hpa-has-replicas.yaml", "Deployment targeted by HPA does not have replicas configured", scorecard.GradeCritical)
+}
+
+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)
+}
diff --git a/score/testdata/deployment-with-hpa-has-replicas.yaml b/score/testdata/deployment-with-hpa-has-replicas.yaml
new file mode 100644
index 0000000..08ad5e8
--- /dev/null
+++ b/score/testdata/deployment-with-hpa-has-replicas.yaml
@@ -0,0 +1,32 @@
+apiVersion: autoscaling/v1
+kind: HorizontalPodAutoscaler
+metadata:
+  name: php-apache
+  namespace: default
+spec:
+  scaleTargetRef:
+    apiVersion: apps/v1
+    kind: Deployment
+    name: php-apache
+  minReplicas: 1
+  maxReplicas: 10
+  metrics:
+    - type: Resource
+      resource:
+        name: cpu
+        target:
+          type: Utilization
+          averageUtilization: 50
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: php-apache
+  namespace: default
+spec:
+  replicas: 123
+  template:
+    spec:
+      containers:
+        - name: foo
+          image: foo:latest
\ No newline at end of file
diff --git a/score/testdata/deployment-with-hpa-not-has-replicas.yaml b/score/testdata/deployment-with-hpa-not-has-replicas.yaml
new file mode 100644
index 0000000..07ae7d3
--- /dev/null
+++ b/score/testdata/deployment-with-hpa-not-has-replicas.yaml
@@ -0,0 +1,31 @@
+apiVersion: autoscaling/v1
+kind: HorizontalPodAutoscaler
+metadata:
+  name: php-apache
+  namespace: default
+spec:
+  scaleTargetRef:
+    apiVersion: apps/v1
+    kind: Deployment
+    name: php-apache
+  minReplicas: 1
+  maxReplicas: 10
+  metrics:
+    - type: Resource
+      resource:
+        name: cpu
+        target:
+          type: Utilization
+          averageUtilization: 50
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: php-apache
+  namespace: default
+spec:
+  template:
+    spec:
+      containers:
+        - name: foo
+          image: foo:latest
\ No newline at end of file