Merge #362

362: Support policy/v1 PodDisruptionBudget r=zegl a=zegl

Support and recommend policy/v1 if using Kubernetes v1.21 or later.

This updates #360 

```
RELNOTE: Support policy/v1 of PodDisruptionBudget. If --kubernetes-version is set to v1.21 or later, policy/v1 will be recommended over policy/v1beta1
```


Co-authored-by: Gustav Westling <[email protected]>

domain/kube-score.go

@@ -6,7 +6,6 @@ import (
 	autoscalingv1 "k8s.io/api/autoscaling/v1"
 	corev1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
-	policyv1beta1 "k8s.io/api/policy/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -128,7 +127,8 @@ type CronJobs interface {
 }
 
 type PodDisruptionBudget interface {
-	PodDisruptionBudget() policyv1beta1.PodDisruptionBudget
+	Namespace() string
+	PodDisruptionBudgetSelector() *metav1.LabelSelector
 	FileLocationer
 }
 

parser/internal/pdb/pod.go

@@ -1,20 +1,43 @@
 package pod
 
 import (
+	policyv1 "k8s.io/api/policy/v1"
 	policyv1beta1 "k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	ks "github.com/zegl/kube-score/domain"
 )
 
-type PodDisruptionBudget struct {
+type PodDisruptionBudgetV1beta1 struct {
 	Obj      policyv1beta1.PodDisruptionBudget
 	Location ks.FileLocation
 }
 
-func (p PodDisruptionBudget) PodDisruptionBudget() policyv1beta1.PodDisruptionBudget {
-	return p.Obj
+func (p PodDisruptionBudgetV1beta1) PodDisruptionBudgetSelector() *metav1.LabelSelector {
+	return p.Obj.Spec.Selector
 }
 
-func (p PodDisruptionBudget) FileLocation() ks.FileLocation {
+func (p PodDisruptionBudgetV1beta1) Namespace() string {
+	return p.Obj.Namespace
+}
+
+func (p PodDisruptionBudgetV1beta1) FileLocation() ks.FileLocation {
+	return p.Location
+}
+
+type PodDisruptionBudgetV1 struct {
+	Obj      policyv1.PodDisruptionBudget
+	Location ks.FileLocation
+}
+
+func (p PodDisruptionBudgetV1) PodDisruptionBudgetSelector() *metav1.LabelSelector {
+	return p.Obj.Spec.Selector
+}
+
+func (p PodDisruptionBudgetV1) FileLocation() ks.FileLocation {
 	return p.Location
 }
+
+func (p PodDisruptionBudgetV1) Namespace() string {
+	return p.Obj.Namespace
+}

parser/parse.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"fmt"
 	"io/ioutil"
+	policyv1 "k8s.io/api/policy/v1"
 	"log"
 	"strings"
 
@@ -322,7 +323,13 @@ func decodeItem(cnf config.Configuration, s *parsedObjects, detectedVersion sche
 	case policyv1beta1.SchemeGroupVersion.WithKind("PodDisruptionBudget"):
 		var disruptBudget policyv1beta1.PodDisruptionBudget
 		errs.AddIfErr(decode(fileContents, &disruptBudget))
-		dbug := internalpdb.PodDisruptionBudget{disruptBudget, fileLocation}
+		dbug := internalpdb.PodDisruptionBudgetV1beta1{disruptBudget, fileLocation}
+		s.podDisruptionBudgets = append(s.podDisruptionBudgets, dbug)
+		s.bothMetas = append(s.bothMetas, ks.BothMeta{disruptBudget.TypeMeta, disruptBudget.ObjectMeta, dbug})
+	case policyv1.SchemeGroupVersion.WithKind("PodDisruptionBudget"):
+		var disruptBudget policyv1.PodDisruptionBudget
+		errs.AddIfErr(decode(fileContents, &disruptBudget))
+		dbug := internalpdb.PodDisruptionBudgetV1{disruptBudget, fileLocation}
 		s.podDisruptionBudgets = append(s.podDisruptionBudgets, dbug)
 		s.bothMetas = append(s.bothMetas, ks.BothMeta{disruptBudget.TypeMeta, disruptBudget.ObjectMeta, dbug})
 

score/disruptionbudget/disruptionbudget.go

@@ -18,13 +18,12 @@ func Register(allChecks *checks.Checks, budgets ks.PodDisruptionBudgets) {
 }
 
 func hasMatching(budgets []ks.PodDisruptionBudget, namespace string, lables map[string]string) (bool, error) {
-	for _, b := range budgets {
-		budget := b.PodDisruptionBudget()
-		if budget.Namespace != namespace {
+	for _, budget := range budgets {
+		if budget.Namespace() != namespace {
 			continue
 		}
 
-		selector, err := metav1.LabelSelectorAsSelector(budget.Spec.Selector)
+		selector, err := metav1.LabelSelectorAsSelector(budget.PodDisruptionBudgetSelector())
 		if err != nil {
 			return false, fmt.Errorf("failed to create selector: %v", err)
 		}

score/poddisruptionbudget_test.go

@@ -8,40 +8,50 @@ import (
 
 func TestStatefulSetPodDisruptionBudgetMatches(t *testing.T) {
 	t.Parallel()
-	testExpectedScore(t, "statefulset-poddisruptionbudget-matches.yaml", "StatefulSet has PodDisruptionBudget", scorecard.GradeAllOK)
+	testExpectedScore(t, "statefulset-poddisruptionbudget-v1beta1-matches.yaml", "StatefulSet has PodDisruptionBudget", scorecard.GradeAllOK)
 }
 
 func TestStatefulSetPodDisruptionBudgetExpressionMatches(t *testing.T) {
 	t.Parallel()
-	testExpectedScore(t, "statefulset-poddisruptionbudget-expression-matches.yaml", "StatefulSet has PodDisruptionBudget", scorecard.GradeAllOK)
+	testExpectedScore(t, "statefulset-poddisruptionbudget-v1beta1-expression-matches.yaml", "StatefulSet has PodDisruptionBudget", scorecard.GradeAllOK)
 }
 
 func TestStatefulSetPodDisruptionBudgetExpressionNoMatch(t *testing.T) {
 	t.Parallel()
-	testExpectedScore(t, "statefulset-poddisruptionbudget-expression-no-match.yaml", "StatefulSet has PodDisruptionBudget", scorecard.GradeCritical)
+	testExpectedScore(t, "statefulset-poddisruptionbudget-v1beta1-expression-no-match.yaml", "StatefulSet has PodDisruptionBudget", scorecard.GradeCritical)
 }
 
 func TestStatefulSetPodDisruptionBudgetNoMatch(t *testing.T) {
 	t.Parallel()
-	testExpectedScore(t, "statefulset-poddisruptionbudget-no-match.yaml", "StatefulSet has PodDisruptionBudget", scorecard.GradeCritical)
+	testExpectedScore(t, "statefulset-poddisruptionbudget-v1beta1-no-match.yaml", "StatefulSet has PodDisruptionBudget", scorecard.GradeCritical)
 }
 
 func TestDeploymentPodDisruptionBudgetMatches(t *testing.T) {
 	t.Parallel()
-	testExpectedScore(t, "deployment-poddisruptionbudget-matches.yaml", "Deployment has PodDisruptionBudget", scorecard.GradeAllOK)
+	testExpectedScore(t, "deployment-poddisruptionbudget-v1beta1-matches.yaml", "Deployment has PodDisruptionBudget", scorecard.GradeAllOK)
 }
 
 func TestDeploymentPodDisruptionBudgetExpressionMatches(t *testing.T) {
 	t.Parallel()
-	testExpectedScore(t, "deployment-poddisruptionbudget-expression-matches.yaml", "Deployment has PodDisruptionBudget", scorecard.GradeAllOK)
+	testExpectedScore(t, "deployment-poddisruptionbudget-v1beta1-expression-matches.yaml", "Deployment has PodDisruptionBudget", scorecard.GradeAllOK)
 }
 
 func TestDeploymentPodDisruptionBudgetExpressionNoMatch(t *testing.T) {
 	t.Parallel()
-	testExpectedScore(t, "deployment-poddisruptionbudget-expression-no-match.yaml", "Deployment has PodDisruptionBudget", scorecard.GradeCritical)
+	testExpectedScore(t, "deployment-poddisruptionbudget-v1beta1-expression-no-match.yaml", "Deployment has PodDisruptionBudget", scorecard.GradeCritical)
 }
 
 func TestDeploymentPodDisruptionBudgetNoMatch(t *testing.T) {
 	t.Parallel()
-	testExpectedScore(t, "deployment-poddisruptionbudget-no-match.yaml", "Deployment has PodDisruptionBudget", scorecard.GradeCritical)
+	testExpectedScore(t, "deployment-poddisruptionbudget-v1beta1-no-match.yaml", "Deployment has PodDisruptionBudget", scorecard.GradeCritical)
+}
+
+func TestDeploymentPodDisruptionBudgetV1Matches(t *testing.T) {
+	t.Parallel()
+	testExpectedScore(t, "deployment-poddisruptionbudget-v1-matches.yaml", "Deployment has PodDisruptionBudget", scorecard.GradeAllOK)
+}
+
+func TestDeploymentPodDisruptionBudgetV1NoMatch(t *testing.T) {
+	t.Parallel()
+	testExpectedScore(t, "deployment-poddisruptionbudget-v1-no-match.yaml", "Deployment has PodDisruptionBudget", scorecard.GradeCritical)
 }

score/stable/stable_version.go

@@ -39,6 +39,9 @@ func metaStableAvailable(kubernetsVersion config.Semver) func(meta domain.BothMe
 			"batch/v1beta1": {
 				"CronJob": recommendedApi{"batch/v1", config.Semver{1, 21}},
 			},
+			"policy/v1beta1": {
+				"PodDisruptionBudget": recommendedApi{"policy/v1", config.Semver{1, 21}},
+			},
 		}
 
 		score.Grade = scorecard.GradeAllOK

score/stable/stable_version_test.go

@@ -30,3 +30,10 @@ func TestStableVersionIngress(t *testing.T) {
 	assert.Equal(t, scorecard.GradeWarning, scoreNew.Grade)
 	assert.Equal(t, []scorecard.TestScoreComment{{Path: "", Summary: "The apiVersion and kind extensions/v1beta1/Ingress is deprecated", Description: "It's recommended to use networking.k8s.io/v1beta1 instead which has been available since Kubernetes v1.14", DocumentationURL: ""}}, scoreNew.Comments)
 }
+
+func TestStableVersionPodDisruptionBudget(t *testing.T) {
+	newKubernetes := metaStableAvailable(config.Semver{1, 21})
+	scoreNew := newKubernetes(ks.BothMeta{TypeMeta: v1.TypeMeta{Kind: "PodDisruptionBudget", APIVersion: "policy/v1beta1"}})
+	assert.Equal(t, scorecard.GradeWarning, scoreNew.Grade)
+	assert.Equal(t, []scorecard.TestScoreComment{{Path: "", Summary: "The apiVersion and kind policy/v1beta1/PodDisruptionBudget is deprecated", Description: "It's recommended to use policy/v1 instead which has been available since Kubernetes v1.21", DocumentationURL: ""}}, scoreNew.Comments)
+}

score/testdata/deployment-poddisruptionbudget-v1-matches.yaml

@@ -0,0 +1,23 @@
+apiVersion: policy/v1
+kind: PodDisruptionBudget
+metadata:
+  name: app-budget
+spec:
+  minAvailable: 2
+  selector:
+    matchLabels:
+      app: foo
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: statefulset-test-1
+spec:
+  template:
+    metadata:
+      labels:
+        app: foo
+    spec:
+      containers:
+      - name: foobar
+        image: foo:bar

score/testdata/deployment-poddisruptionbudget-v1-no-match.yaml

@@ -0,0 +1,23 @@
+apiVersion: policy/v1
+kind: PodDisruptionBudget
+metadata:
+  name: app-budget
+spec:
+  minAvailable: 2
+  selector:
+    matchLabels:
+      app: not-foo
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: statefulset-test-1
+spec:
+  template:
+    metadata:
+      labels:
+        app: foo
+    spec:
+      containers:
+      - name: foobar
+        image: foo:bar