diff --git a/internal/issues/assets/codes.yml b/internal/issues/assets/codes.yml index 29e5059f..f42e6a99 100644 --- a/internal/issues/assets/codes.yml +++ b/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: diff --git a/internal/issues/codes_test.go b/internal/issues/codes_test.go index 4a04d7e8..3c8bf10e 100644 --- a/internal/issues/codes_test.go +++ b/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) } diff --git a/internal/sanitize/pod.go b/internal/sanitize/pod.go index 16b0aec4..81bcf246 100644 --- a/internal/sanitize/pod.go +++ b/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, ", ")) + } +} diff --git a/internal/sanitize/pod_test.go b/internal/sanitize/pod_test.go index a08f0c45..840722da 100644 --- a/internal/sanitize/pod_test.go +++ b/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()[""]) + }) + } +}