Skip to content

Commit

Permalink
DVO-128: enable non-isolated Pod check & DVO-133 fix and enhance grou… (
Browse files Browse the repository at this point in the history 
#262)

* DVO-128: enable non-isolated Pod check & DVO-133 fix and enhance group matching

* update after review
  • Loading branch information
@tremes
tremes authored Jul 11, 2023
1 parent 4aa01dd commit 2141ef3
Show file tree
Hide file tree
Showing 8 changed files with 472 additions and 323 deletions.
59 changes: 30 additions & 29 deletions pkg/controller/generic_reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/discovery"
Expand Down Expand Up @@ -170,7 +171,8 @@ func (gr *GenericReconciler) groupAppObjects(ctx context.Context,

for i := range list.Items {
obj := &list.Items[i]
processObjectLabelSelectors(obj, relatedObjects)
processResourceLabels(obj, relatedObjects)
gr.processResourceSelectors(obj, relatedObjects)
}

listContinue := list.GetContinue()
Expand All @@ -183,41 +185,40 @@ func (gr *GenericReconciler) groupAppObjects(ctx context.Context,
return relatedObjects, nil
}

// processObjectLabelSelectors gets 'app' label selectors from the respective object and parses them and stores
// them in the 'relatedObjects' map.
func processObjectLabelSelectors(obj *unstructured.Unstructured,
// processResourceLabels reads resource labels and if the labels
// are not empty then format them into string and put the string value
// as key and the object as a value into "relatedObjects" map
func processResourceLabels(obj *unstructured.Unstructured,
relatedObjects map[string][]*unstructured.Unstructured) {

// if the object has an empty non-null selector the add it to every known group
if utils.HasEmptySelector(obj) {
for k := range relatedObjects {
relatedObjects[k] = append(relatedObjects[k], obj)
}
objLabels := utils.GetLabels(obj)
if len(objLabels) == 0 {
return
}
appSelectors, err := utils.GetAppSelectors(obj)
labelsString := labels.FormatLabels(objLabels)
relatedObjects[labelsString] = append(relatedObjects[labelsString], obj)
}

// processResourceSelectors reads resource selector and then tries to match
// the selector to known labels (keys in the relatedObjects map). If a match is found then
// the object is added to the corresponding group (values in the relatedObjects map).
func (gr *GenericReconciler) processResourceSelectors(obj *unstructured.Unstructured,
relatedObjects map[string][]*unstructured.Unstructured) {
labelSelector := utils.GetLabelSelector(obj)
selector, err := metav1.LabelSelectorAsSelector(labelSelector)
if err != nil {
// swallow the error here. it will be too noisy to log
gr.logger.Error(err, "cannot convert label selector for object", obj.GetKind(), obj.GetName())
return
}
for _, as := range appSelectors {
switch as.Operator {
case metav1.LabelSelectorOpExists:
for k := range relatedObjects {
relatedObjects[k] = append(relatedObjects[k], obj)
}
case metav1.LabelSelectorOpIn:
for v := range as.Values {
relatedObjects[v] = append(relatedObjects[v], obj)
}
case metav1.LabelSelectorOpNotIn:
for selectorVal := range as.Values {
for appLabelVal := range relatedObjects {
if appLabelVal != selectorVal {
relatedObjects[appLabelVal] = append(relatedObjects[appLabelVal], obj)
}
}
}

for k := range relatedObjects {
labelsSet, err := labels.ConvertSelectorToLabelsMap(k)
if err != nil {
gr.logger.Error(err, "cannot convert selector to labels map for", obj.GetKind(), obj.GetName())
continue
}
if selector.Matches(labelsSet) {
relatedObjects[k] = append(relatedObjects[k], obj)
}
}
}
Expand Down
108 changes: 97 additions & 11 deletions pkg/controller/generic_reconciler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/assert"
appsv1 "k8s.io/api/apps/v1"
v1 "k8s.io/api/core/v1"
networkingv1 "k8s.io/api/networking/v1"
policyv1 "k8s.io/api/policy/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand Down Expand Up @@ -137,7 +138,7 @@ func TestGroupAppObjects(t *testing.T) {
},
},
expectedNames: map[string][]string{
"B": {"test-B-deployment"},
"app=B": {"test-B-deployment"},
},
},
{
Expand Down Expand Up @@ -202,8 +203,8 @@ func TestGroupAppObjects(t *testing.T) {
Version: "v1",
}},
expectedNames: map[string][]string{
"A": {"test-A-pdb", "test-A-deployment"},
"B": {"test-B-pod", "test-B-deployment"},
"app=A": {"test-A-pdb", "test-A-deployment"},
"app=B": {"test-B-pod", "test-B-deployment"},
},
},
{
Expand Down Expand Up @@ -327,10 +328,10 @@ func TestGroupAppObjects(t *testing.T) {
},
},
expectedNames: map[string][]string{
"A": {"test-pdb-A-B-C", "test-pdb-not-in-C", "test-pdb-exists", "test-deployment-A", "test-pdb-not-in-D"}, //nolint:lll
"B": {"test-pdb-A-B-C", "test-pdb-not-in-C", "test-pdb-exists", "test-deployment-B", "test-pdb-not-in-D"}, //nolint:lll
"C": {"test-pdb-A-B-C", "test-pdb-exists", "test-deployment-C", "test-pdb-not-in-D"},
"D": {"test-deployment-D", "test-pdb-exists", "test-pdb-not-in-C"},
"app=A": {"test-pdb-A-B-C", "test-pdb-not-in-C", "test-pdb-exists", "test-deployment-A", "test-pdb-not-in-D"}, //nolint:lll
"app=B": {"test-pdb-A-B-C", "test-pdb-not-in-C", "test-pdb-exists", "test-deployment-B", "test-pdb-not-in-D"}, //nolint:lll
"app=C": {"test-pdb-A-B-C", "test-pdb-exists", "test-deployment-C", "test-pdb-not-in-D"}, //nolint:lll
"app=D": {"test-deployment-D", "test-pdb-exists", "test-pdb-not-in-C"},
},
},
{
Expand Down Expand Up @@ -403,8 +404,8 @@ func TestGroupAppObjects(t *testing.T) {
},
},
expectedNames: map[string][]string{
"A": {"statefulset-A", "pdb-not-in-B"},
"B": {"statefulset-B", "pdb-not-in-A"},
"app=A": {"statefulset-A", "pdb-not-in-B"},
"app=B": {"statefulset-B", "pdb-not-in-A"},
},
},
{
Expand Down Expand Up @@ -452,8 +453,92 @@ func TestGroupAppObjects(t *testing.T) {
},
},
expectedNames: map[string][]string{
"A": {"app-A", "test-pdb"},
"B": {"app-B", "test-pdb"},
"app=A": {"app-A", "test-pdb"},
"app=B": {"app-B", "test-pdb"},
},
},
{
name: "Deployment with matching NetworkPolicy",
namespace: "test",
objs: []client.Object{
&networkingv1.NetworkPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "np-A",
Namespace: "test",
},
Spec: networkingv1.NetworkPolicySpec{
PodSelector: metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: "app",
Operator: metav1.LabelSelectorOpNotIn,
Values: []string{"B"},
},
},
},
},
},
&appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: "deployment-A",
Namespace: "test",
Labels: map[string]string{
"app": "A",
},
},
},
},
gvks: []schema.GroupVersionKind{
{
Group: "apps",
Kind: "Deployment",
Version: "v1",
},
{
Group: "networking.k8s.io",
Kind: "NetworkPolicy",
Version: "v1",
},
},
expectedNames: map[string][]string{
"app=A": {"deployment-A", "np-A"},
},
},
{
name: "Deployment with non-matching NetworkPolicy (no selector)",
namespace: "test",
objs: []client.Object{
&networkingv1.NetworkPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "np-A",
Namespace: "test",
},
Spec: networkingv1.NetworkPolicySpec{},
},
&appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: "deployment-A",
Namespace: "test",
Labels: map[string]string{
"app": "A",
},
},
},
},
gvks: []schema.GroupVersionKind{
{
Group: "apps",
Kind: "Deployment",
Version: "v1",
},
{
Group: "networking.k8s.io",
Kind: "NetworkPolicy",
Version: "v1",
},
},
expectedNames: map[string][]string{
"app=A": {"deployment-A"},
},
},
}
Expand All @@ -465,6 +550,7 @@ func TestGroupAppObjects(t *testing.T) {
assert.NoError(t, err)
groupMap, err := gr.groupAppObjects(context.Background(), tt.namespace, tt.gvks)
assert.NoError(t, err)

for expectedLabel, expectedNames := range tt.expectedNames {
objects, ok := groupMap[expectedLabel]
assert.True(t, ok, "can't find label %s", expectedLabel)
Expand Down
113 changes: 0 additions & 113 deletions pkg/utils/app_label.go
View file

This file was deleted.

Loading

0 comments on commit 2141ef3

Please sign in to comment.