Merge pull request #224 from mercedes-benz/add-check-pod-multiple-pdb…

…-usage

Add check for pods that are managed by multiple pdbs

internal/issues/assets/codes.yml

@@ -71,6 +71,9 @@ codes:
   208:
     message: Unmanaged pod detected. Best to use a controller
     severity: 2
+  209:
+    message: Pod is managed by multiple PodDisruptionBudgets (%s)
+    severity: 2
 
   # Security
   300:

internal/issues/codes_test.go

@@ -12,7 +12,7 @@ func TestCodesLoad(t *testing.T) {
 	cc, err := issues.LoadCodes()
 
 	assert.Nil(t, err)
-	assert.Equal(t, 85, len(cc.Glossary))
+	assert.Equal(t, 86, len(cc.Glossary))
 	assert.Equal(t, "No liveness probe", cc.Glossary[103].Message)
 	assert.Equal(t, config.WarnLevel, cc.Glossary[103].Severity)
 }

internal/sanitize/pod.go

@@ -2,6 +2,11 @@ package sanitize
 
 import (
 	"context"
+	"sort"
+	"strings"
+
+	"github.com/rs/zerolog/log"
+	"k8s.io/apimachinery/pkg/labels"
 
 	"github.com/derailed/popeye/internal"
 	"github.com/derailed/popeye/internal/cache"
@@ -73,6 +78,7 @@ func NewPod(co *issues.Collector, lister PodMXLister) *Pod {
 // Sanitize cleanse the resource..
 func (p *Pod) Sanitize(ctx context.Context) error {
 	mx := p.ListPodsMetrics()
+	pdbs := p.ListPodDisruptionBudgets()
 	for fqn, po := range p.ListPods() {
 		p.InitOutcome(fqn)
 		ctx = internal.WithFQN(ctx, fqn)
@@ -86,6 +92,7 @@ func (p *Pod) Sanitize(ctx context.Context) error {
 		if !ownedByDaemonSet(po) {
 			p.checkPdb(ctx, po.ObjectMeta.Labels)
 		}
+		p.checkForMultiplePdbMatches(ctx, po.Namespace, po.ObjectMeta.Labels, pdbs)
 		p.checkSecure(ctx, fqn, po.Spec)
 		pmx, cmx := mx[fqn], client.ContainerMetrics{}
 		containerMetrics(pmx, cmx)
@@ -285,3 +292,25 @@ func isPartOfJob(po *v1.Pod) bool {
 
 	return false
 }
+
+func (p *Pod) checkForMultiplePdbMatches(ctx context.Context, podNamespace string, podLabels map[string]string, pdbs map[string]*polv1beta1.PodDisruptionBudget) {
+	var matchedPdbs []string
+	for _, pdb := range pdbs {
+		if podNamespace != pdb.Namespace {
+			continue
+		}
+		selector, err := metav1.LabelSelectorAsSelector(pdb.Spec.Selector)
+		if err != nil {
+			log.Error().Err(err).Msg("No selectors found")
+			return
+		}
+		if selector.Empty() || !selector.Matches(labels.Set(podLabels)) {
+			continue
+		}
+		matchedPdbs = append(matchedPdbs, pdb.Name)
+	}
+	if len(matchedPdbs) > 1 {
+		sort.Strings(matchedPdbs)
+		p.AddCode(ctx, 209, strings.Join(matchedPdbs, ", "))
+	}
+}

internal/sanitize/pod_test.go

@@ -1,6 +1,7 @@
 package sanitize
 
 import (
+	"context"
 	"testing"
 
 	"github.com/derailed/popeye/internal"
@@ -395,3 +396,178 @@ func makeSecPod(pod, init, co1, co2 NonRootUser) v1.Pod {
 		},
 	}
 }
+
+func TestPodCheckForMultiplePdbMatches(t *testing.T) {
+	type fields struct {
+		Collector   *issues.Collector
+		PodMXLister PodMXLister
+	}
+	type args struct {
+		ctx          context.Context
+		podLabels    map[string]string
+		podNamespace string
+		pdbs         map[string]*polv1beta1.PodDisruptionBudget
+	}
+	tests := []struct {
+		name   string
+		fields fields
+		args   args
+		want   issues.Issues
+	}{
+		{
+			name: "pod with one label - two pdb matches",
+			args: args{
+				podNamespace: "namespace-1",
+				podLabels:    map[string]string{"app": "test"},
+				pdbs: map[string]*polv1beta1.PodDisruptionBudget{
+					"pdb": {
+						Spec: polv1beta1.PodDisruptionBudgetSpec{
+							Selector: &metav1.LabelSelector{
+								MatchLabels: map[string]string{"app": "test"},
+							},
+						},
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "pdb-1",
+							Namespace: "namespace-1",
+						},
+					},
+					"pdb2": {
+						Spec: polv1beta1.PodDisruptionBudgetSpec{
+							Selector: &metav1.LabelSelector{
+								MatchLabels: map[string]string{"app": "test"},
+							},
+						},
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "pdb-2",
+							Namespace: "namespace-1",
+						},
+					},
+				},
+			},
+			want: issues.Issues{
+				issues.Issue{
+					GVR:     "v1/pods",
+					Group:   "__root__",
+					Level:   2,
+					Message: "[POP-209] Pod is managed by multiple PodDisruptionBudgets (pdb-1, pdb-2)"},
+			},
+		},
+		{
+			name: "pod with one label - three pdbs - only two in pod namespace - expecting two matches",
+			args: args{
+				podNamespace: "namespace-1",
+				podLabels:    map[string]string{"app": "test"},
+				pdbs: map[string]*polv1beta1.PodDisruptionBudget{
+					"pdb": {
+						Spec: polv1beta1.PodDisruptionBudgetSpec{
+							Selector: &metav1.LabelSelector{
+								MatchLabels: map[string]string{"app": "test"},
+							},
+						},
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "pdb-1",
+							Namespace: "namespace-1",
+						},
+					},
+					"pdb2": {
+						Spec: polv1beta1.PodDisruptionBudgetSpec{
+							Selector: &metav1.LabelSelector{
+								MatchLabels: map[string]string{"app": "test"},
+							},
+						},
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "pdb-2",
+							Namespace: "namespace-1",
+						},
+					},
+					"pdb3": {
+						Spec: polv1beta1.PodDisruptionBudgetSpec{
+							Selector: &metav1.LabelSelector{
+								MatchLabels: map[string]string{"app": "test"},
+							},
+						},
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "pdb-3",
+							Namespace: "namespace-2",
+						},
+					},
+				},
+			},
+			want: issues.Issues{
+				issues.Issue{
+					GVR:     "v1/pods",
+					Group:   "__root__",
+					Level:   2,
+					Message: "[POP-209] Pod is managed by multiple PodDisruptionBudgets (pdb-1, pdb-2)"},
+			},
+		},
+		{
+			name: "one pdb match, no issue expected",
+			args: args{
+				podNamespace: "namespace-1",
+				podLabels:    map[string]string{"app": "test", "app2": "test2"},
+				pdbs: map[string]*polv1beta1.PodDisruptionBudget{
+					"pdb": {
+						Spec: polv1beta1.PodDisruptionBudgetSpec{
+							Selector: &metav1.LabelSelector{
+								MatchLabels: map[string]string{"app": "test", "app2": "test2"},
+							},
+						},
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "pdb-1",
+							Namespace: "namespace-1",
+						},
+					},
+					"pdb2": {
+						Spec: polv1beta1.PodDisruptionBudgetSpec{
+							Selector: &metav1.LabelSelector{
+								MatchLabels: map[string]string{"app3": "test3"},
+							},
+						},
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "pdb-2",
+							Namespace: "namespace-1",
+						},
+					},
+				},
+			},
+			want: issues.Issues(nil),
+		},
+		{
+			name: "pod with no label - no issue expected",
+			args: args{
+				podLabels: map[string]string{},
+				pdbs: map[string]*polv1beta1.PodDisruptionBudget{
+					"pdb": {
+						Spec: polv1beta1.PodDisruptionBudgetSpec{
+							Selector: &metav1.LabelSelector{
+								MatchLabels: map[string]string{"app": "test"},
+							},
+						},
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "pdb-1"},
+					},
+					"pdb2": {
+						Spec: polv1beta1.PodDisruptionBudgetSpec{
+							Selector: &metav1.LabelSelector{
+								MatchLabels: map[string]string{"app": "test"},
+							},
+						},
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "pdb-2"},
+					},
+				},
+			},
+			want: issues.Issues(nil),
+		},
+	}
+	ctx := makeContext("v1/pods", "po")
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			p := NewPod(issues.NewCollector(loadCodes(t), makeConfig(t)), tt.fields.PodMXLister)
+
+			p.checkForMultiplePdbMatches(ctx, tt.args.podNamespace, tt.args.podLabels, tt.args.pdbs)
+			assert.Equal(t, tt.want, p.Outcome()[""])
+		})
+	}
+}