Refactor EgressNetworkPolicyReconciler , `InternetNetworkPolicyReco…

…nciler`, `PortNetworkPolicyReconciler` and `PortEgressNetworkPolicyReconciler` to reconcile `ServiceEffectivePolicy` instead of `ClientIntents` (#339)

Co-authored-by: Ori Shoshan <ori@otterize.com>

src/operator/controllers/intents_controller.go

@@ -22,10 +22,6 @@ import (
 	otterizev1alpha3 "github.com/otterize/intents-operator/src/operator/api/v1alpha3"
 	"github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers"
 	"github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers/database"
-	"github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers/egress_network_policy"
-	"github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers/internet_network_policy"
-	"github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers/port_egress_network_policy"
-	"github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers/port_network_policy"
 	"github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers/protected_services"
 	"github.com/otterize/intents-operator/src/operator/controllers/kafkaacls"
 	"github.com/otterize/intents-operator/src/shared/errors"
@@ -76,9 +72,6 @@ func NewIntentsReconciler(
 	client client.Client,
 	scheme *runtime.Scheme,
 	kafkaServerStore kafkaacls.ServersStore,
-	portNetpolReconciler *port_network_policy.PortNetworkPolicyReconciler,
-	egressNetpolReconciler *egress_network_policy.EgressNetworkPolicyReconciler,
-	portEgressNetpolReconciler *port_egress_network_policy.PortEgressNetworkPolicyReconciler,
 	restrictToNamespaces []string,
 	enforcementConfig EnforcementConfig,
 	otterizeClient operator_cloud_client.CloudClient,
@@ -105,8 +98,6 @@ func NewIntentsReconciler(
 		reconcilers...,
 	)
 
-	reconcilersGroup.AddToGroup(portNetpolReconciler)
-
 	intentsReconciler := &IntentsReconciler{
 		group:  reconcilersGroup,
 		client: client,
@@ -127,13 +118,6 @@ func NewIntentsReconciler(
 		intentsReconciler.group.AddToGroup(databaseReconciler)
 	}
 
-	if enforcementConfig.EnableEgressNetworkPolicyReconcilers {
-		internetNetpolReconciler := internet_network_policy.NewInternetNetworkPolicyReconciler(client, scheme, restrictToNamespaces, enforcementConfig.EnableNetworkPolicy, enforcementConfig.EnforcementDefaultState)
-		intentsReconciler.group.AddToGroup(internetNetpolReconciler)
-		intentsReconciler.group.AddToGroup(egressNetpolReconciler)
-		intentsReconciler.group.AddToGroup(portEgressNetpolReconciler)
-	}
-
 	return intentsReconciler
 }
 

src/operator/controllers/intents_controller_test.go

@@ -28,9 +28,6 @@ func (s *IntentsControllerTestSuite) SetupTest() {
 		scheme.Scheme,
 		nil,
 		nil,
-		nil,
-		nil,
-		nil,
 		EnforcementConfig{},
 		nil,
 		"",

src/operator/controllers/intents_reconcilers/egress_network_policy/egress_network_policy_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers"
 	"github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers/consts"
 	mocks "github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers/mocks"
+	"github.com/otterize/intents-operator/src/operator/effectivepolicy"
 	"github.com/otterize/intents-operator/src/shared/testbase"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/viper"
@@ -24,19 +25,14 @@ import (
 )
 
 const (
-	protectedServicesResourceName        = "staging-protected-services"
-	protectedService                     = "test-service"
-	protectedServiceFormattedName        = "test-service-test-namespace-b0207e"
-	anotherProtectedServiceResourceName  = "protect-other-services"
-	anotherProtectedService              = "other-test-service"
-	anotherProtectedServiceFormattedName = "other-test-service-test-namespace-398a04"
-	testServerNamespace                  = "test-server-namespace"
-	testClientNamespace                  = "test-client-namespace"
+	testServerNamespace = "test-server-namespace"
+	testClientNamespace = "test-client-namespace"
 )
 
 type EgressNetworkPolicyReconcilerTestSuite struct {
 	testbase.MocksSuiteBase
 	Reconciler            *EgressNetworkPolicyReconciler
+	EPIntentsReconciler   *intents_reconcilers.ServiceEffectivePolicyIntentsReconciler
 	externalNetpolHandler *mocks.MockexternalNetpolHandler
 }
 
@@ -49,15 +45,25 @@ func (s *EgressNetworkPolicyReconcilerTestSuite) SetupTest() {
 	s.externalNetpolHandler = mocks.NewMockexternalNetpolHandler(s.Controller)
 	restrictToNamespaces := make([]string, 0)
 
+	scheme := &runtime.Scheme{}
 	s.Reconciler = NewEgressNetworkPolicyReconciler(
 		s.Client,
-		&runtime.Scheme{},
+		scheme,
 		restrictToNamespaces,
 		true,
 		true,
 	)
 
 	s.Reconciler.Recorder = s.Recorder
+
+	epReconciler := effectivepolicy.NewGroupReconciler(s.Client,
+		scheme, s.Reconciler)
+	s.EPIntentsReconciler = intents_reconcilers.NewServiceEffectiveIntentsReconciler(s.Client,
+		scheme, epReconciler)
+
+	s.Reconciler.Recorder = s.Recorder
+	epReconciler.InjectableRecorder.Recorder = s.Recorder
+	s.EPIntentsReconciler.Recorder = s.Recorder
 }
 
 func (s *EgressNetworkPolicyReconcilerTestSuite) TearDownTest() {
@@ -67,6 +73,55 @@ func (s *EgressNetworkPolicyReconcilerTestSuite) TearDownTest() {
 	s.MocksSuiteBase.TearDownTest()
 }
 
+func (s *EgressNetworkPolicyReconcilerTestSuite) expectGetAllEffectivePolicies(clientIntents []otterizev1alpha3.ClientIntents) {
+	var intentsList otterizev1alpha3.ClientIntentsList
+
+	s.Client.EXPECT().List(gomock.Any(), &intentsList).DoAndReturn(func(_ context.Context, intents *otterizev1alpha3.ClientIntentsList, _ ...any) error {
+		intents.Items = append(intents.Items, clientIntents...)
+		return nil
+	})
+
+	// create service to ClientIntents pointing to it
+	services := make(map[string][]otterizev1alpha3.ClientIntents)
+	for _, clientIntent := range clientIntents {
+		for _, intentCall := range clientIntent.GetCallsList() {
+			server := otterizev1alpha3.GetFormattedOtterizeIdentity(intentCall.GetTargetServerName(), intentCall.GetTargetServerNamespace(clientIntent.Namespace))
+			services[server] = append(services[server], clientIntent)
+		}
+	}
+
+	matchFieldsPtr := &client.MatchingFields{}
+	s.Client.EXPECT().List(
+		gomock.Any(),
+		&otterizev1alpha3.ClientIntentsList{},
+		gomock.AssignableToTypeOf(matchFieldsPtr),
+	).DoAndReturn(func(_ context.Context, intents *otterizev1alpha3.ClientIntentsList, args ...any) error {
+		matchFields := args[0].(*client.MatchingFields)
+		intents.Items = services[(*matchFields)[otterizev1alpha3.OtterizeFormattedTargetServerIndexField]]
+		return nil
+	}).AnyTimes()
+
+}
+
+func (s *EgressNetworkPolicyReconcilerTestSuite) expectRemoveOrphanFindsPolicies(netpols []v1.NetworkPolicy) {
+	selector, err := metav1.LabelSelectorAsSelector(&metav1.LabelSelector{MatchExpressions: []metav1.LabelSelectorRequirement{
+		{
+			Key:      otterizev1alpha3.OtterizeEgressNetworkPolicy,
+			Operator: metav1.LabelSelectorOpExists,
+		},
+	}})
+	s.Require().NoError(err)
+
+	s.Client.EXPECT().List(
+		gomock.Any(), gomock.Eq(&v1.NetworkPolicyList{}), &client.ListOptions{LabelSelector: selector},
+	).DoAndReturn(
+		func(_ context.Context, netpolList *v1.NetworkPolicyList, _ ...any) error {
+			netpolList.Items = append(netpolList.Items, netpols...)
+			return nil
+		},
+	)
+}
+
 func (s *EgressNetworkPolicyReconcilerTestSuite) TestCreateNetworkPolicy() {
 	clientIntentsName := "client-intents"
 	policyName := "egress-to-test-server.test-server-namespace-from-test-client"
@@ -170,14 +225,9 @@ func (s *EgressNetworkPolicyReconcilerTestSuite) TestUpdateNetworkPolicy() {
 		},
 	}
 
-	// Initial call to get the ClientIntents object when reconciler starts
-	emptyIntents := &otterizev1alpha3.ClientIntents{}
-	s.Client.EXPECT().Get(gomock.Any(), req.NamespacedName, gomock.Eq(emptyIntents)).DoAndReturn(
-		func(ctx context.Context, name types.NamespacedName, intents *otterizev1alpha3.ClientIntents, options ...client.ListOption) error {
-			intents.Namespace = testClientNamespace
-			intents.Spec = intentsSpec
-			return nil
-		})
+	clientIntents := otterizev1alpha3.ClientIntents{Spec: intentsSpec}
+	clientIntents.Namespace = testClientNamespace
+	clientIntents.Name = clientIntentsName
 
 	// Search for existing NetworkPolicy
 	emptyNetworkPolicy := &v1.NetworkPolicy{}
@@ -204,8 +254,9 @@ func (s *EgressNetworkPolicyReconcilerTestSuite) TestUpdateNetworkPolicy() {
 	// Update NetworkPolicy
 	s.Client.EXPECT().Patch(gomock.Any(), gomock.Eq(newPolicy), intents_reconcilers.MatchPatch(client.MergeFrom(existingBadPolicy))).Return(nil)
 	s.ignoreRemoveOrphan()
+	s.expectGetAllEffectivePolicies([]otterizev1alpha3.ClientIntents{clientIntents})
 
-	res, err := s.Reconciler.Reconcile(context.Background(), req)
+	res, err := s.EPIntentsReconciler.Reconcile(context.Background(), req)
 	s.NoError(err)
 	s.Empty(res)
 	s.ExpectEvent(consts.ReasonCreatedEgressNetworkPolicies)
@@ -243,14 +294,6 @@ func (s *EgressNetworkPolicyReconcilerTestSuite) TestRemoveOrphanNetworkPolicy()
 		Spec: intentsSpec,
 	}
 
-	// Initial call to get the ClientIntents object when reconciler starts
-	emptyIntents := &otterizev1alpha3.ClientIntents{}
-	s.Client.EXPECT().Get(gomock.Any(), req.NamespacedName, gomock.Eq(emptyIntents)).DoAndReturn(
-		func(ctx context.Context, name types.NamespacedName, intents *otterizev1alpha3.ClientIntents, options ...client.ListOption) error {
-			clientIntents.DeepCopyInto(intents)
-			return nil
-		})
-
 	// Search for existing NetworkPolicy
 	emptyNetworkPolicy := &v1.NetworkPolicy{}
 	networkPolicyNamespacedName := types.NamespacedName{
@@ -293,29 +336,9 @@ func (s *EgressNetworkPolicyReconcilerTestSuite) TestRemoveOrphanNetworkPolicy()
 			return nil
 		})
 
-	serverLabelSelector := client.MatchingFields{otterizev1alpha3.OtterizeFormattedTargetServerIndexField: existingPolicy.Labels[otterizev1alpha3.OtterizeEgressNetworkPolicyTarget]}
-	s.Client.EXPECT().List(
-		gomock.Any(),
-		gomock.Eq(&otterizev1alpha3.ClientIntentsList{}),
-		&serverLabelSelector).DoAndReturn(
-		func(ctx context.Context, list *otterizev1alpha3.ClientIntentsList, options ...client.ListOption) error {
-			list.Items = []otterizev1alpha3.ClientIntents{clientIntents}
-			return nil
-		})
-
-	orphanServerLabelSelector := client.MatchingFields{otterizev1alpha3.OtterizeFormattedTargetServerIndexField: orphanPolicy.Labels[otterizev1alpha3.OtterizeEgressNetworkPolicyTarget]}
-	s.Client.EXPECT().List(
-		gomock.Any(),
-		gomock.Eq(&otterizev1alpha3.ClientIntentsList{}),
-		&orphanServerLabelSelector).DoAndReturn(
-		func(ctx context.Context, list *otterizev1alpha3.ClientIntentsList, options ...client.ListOption) error {
-			// There are no ClientIntents for this NetworkPolicy, so it should be deleted
-			list.Items = []otterizev1alpha3.ClientIntents{}
-			return nil
-		})
-
 	s.Client.EXPECT().Delete(gomock.Any(), gomock.Eq(orphanPolicy)).Return(nil)
-	res, err := s.Reconciler.Reconcile(context.Background(), req)
+	s.expectGetAllEffectivePolicies([]otterizev1alpha3.ClientIntents{clientIntents})
+	res, err := s.EPIntentsReconciler.Reconcile(context.Background(), req)
 	s.NoError(err)
 	s.Empty(res)
 	s.ExpectEvent(consts.ReasonCreatedEgressNetworkPolicies)
@@ -340,8 +363,6 @@ func (s *EgressNetworkPolicyReconcilerTestSuite) testCleanNetworkPolicy(clientIn
 		},
 	}
 
-	// Initial call to get the ClientIntents object when reconciler starts
-	emptyIntents := &otterizev1alpha3.ClientIntents{}
 	clientIntentsObj := otterizev1alpha3.ClientIntents{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:              clientIntentsName,
@@ -350,23 +371,13 @@ func (s *EgressNetworkPolicyReconcilerTestSuite) testCleanNetworkPolicy(clientIn
 		},
 		Spec: intentsSpec,
 	}
-
-	s.Client.EXPECT().Get(gomock.Any(), req.NamespacedName, gomock.Eq(emptyIntents)).DoAndReturn(
-		func(ctx context.Context, name types.NamespacedName, intents *otterizev1alpha3.ClientIntents, options ...client.ListOption) error {
-			clientIntentsObj.DeepCopyInto(intents)
-			return nil
-		})
+	s.expectGetAllEffectivePolicies([]otterizev1alpha3.ClientIntents{clientIntentsObj})
 
 	// Remove network policy:
 	// 1. get the network policy
 	// 2. delete it
 	// 3.call external netpol handler
 
-	networkPolicyNamespacedName := types.NamespacedName{
-		Namespace: serverNamespace,
-		Name:      policyName,
-	}
-
 	existingPolicy := networkPolicyTemplate(
 		policyName,
 		clientNamespace,
@@ -375,17 +386,10 @@ func (s *EgressNetworkPolicyReconcilerTestSuite) testCleanNetworkPolicy(clientIn
 		testServerNamespace,
 	)
 
-	emptyNetworkPolicy := &v1.NetworkPolicy{}
-	s.Client.EXPECT().Get(gomock.Any(), networkPolicyNamespacedName, gomock.Eq(emptyNetworkPolicy)).DoAndReturn(
-		func(ctx context.Context, name types.NamespacedName, networkPolicy *v1.NetworkPolicy, options ...client.ListOption) error {
-			existingPolicy.DeepCopyInto(networkPolicy)
-			return nil
-		})
-
 	s.Client.EXPECT().Delete(gomock.Any(), gomock.Eq(existingPolicy)).Return(nil)
 
-	s.Client.EXPECT().Update(gomock.Any(), gomock.Eq(&clientIntentsObj)).Return(nil)
-	res, err := s.Reconciler.Reconcile(context.Background(), req)
+	s.expectRemoveOrphanFindsPolicies([]v1.NetworkPolicy{*existingPolicy})
+	res, err := s.EPIntentsReconciler.Reconcile(context.Background(), req)
 	s.NoError(err)
 	s.Empty(res)
 }
@@ -420,13 +424,9 @@ func (s *EgressNetworkPolicyReconcilerTestSuite) testCreateNetworkPolicy(
 	}
 
 	// Initial call to get the ClientIntents object when reconciler starts
-	emptyIntents := &otterizev1alpha3.ClientIntents{}
-	s.Client.EXPECT().Get(gomock.Any(), req.NamespacedName, gomock.Eq(emptyIntents)).DoAndReturn(
-		func(ctx context.Context, name types.NamespacedName, intents *otterizev1alpha3.ClientIntents, options ...client.ListOption) error {
-			intents.Namespace = clientNamespace
-			intents.Spec = intentsSpec
-			return nil
-		})
+	clientIntents := otterizev1alpha3.ClientIntents{Spec: intentsSpec}
+	clientIntents.Namespace = clientNamespace
+	clientIntents.Name = clientIntentsName
 
 	if defaultEnforcementState == false {
 		s.Client.EXPECT().List(gomock.Any(), gomock.Eq(&otterizev1alpha3.ProtectedServiceList{}), gomock.Any()).DoAndReturn(
@@ -459,7 +459,8 @@ func (s *EgressNetworkPolicyReconcilerTestSuite) testCreateNetworkPolicy(
 
 	s.ignoreRemoveOrphan()
 
-	res, err := s.Reconciler.Reconcile(context.Background(), req)
+	s.expectGetAllEffectivePolicies([]otterizev1alpha3.ClientIntents{clientIntents})
+	res, err := s.EPIntentsReconciler.Reconcile(context.Background(), req)
 	s.NoError(err)
 	s.Empty(res)
 }
@@ -565,18 +566,14 @@ func (s *EgressNetworkPolicyReconcilerTestSuite) testEnforcementDisabled() {
 		},
 	}
 
-	// Initial call to get the ClientIntents object when reconciler starts
-	emptyIntents := &otterizev1alpha3.ClientIntents{}
-	s.Client.EXPECT().Get(gomock.Any(), req.NamespacedName, gomock.Eq(emptyIntents)).DoAndReturn(
-		func(ctx context.Context, name types.NamespacedName, intents *otterizev1alpha3.ClientIntents, options ...client.ListOption) error {
-			intents.Namespace = testClientNamespace
-			intents.Spec = intentsSpec
-			return nil
-		})
+	clientIntents := otterizev1alpha3.ClientIntents{Spec: intentsSpec}
+	clientIntents.Name = clientIntentsName
+	clientIntents.Namespace = testClientNamespace
 
 	s.ignoreRemoveOrphan()
+	s.expectGetAllEffectivePolicies([]otterizev1alpha3.ClientIntents{clientIntents})
 
-	res, err := s.Reconciler.Reconcile(context.Background(), req)
+	res, err := s.EPIntentsReconciler.Reconcile(context.Background(), req)
 	s.NoError(err)
 	s.Empty(res)
 }

src/operator/controllers/intents_reconcilers/ingress_network_policy/ingress_network_policy.go

@@ -219,6 +219,12 @@ func (r *IngressNetpolEffectivePolicyReconciler) removeNetworkPoliciesThatShould
 		}
 	}
 
+	deletedCount := len(networkPolicyList.Items) - netpolNamesThatShouldExist.Len()
+	if deletedCount > 0 {
+		telemetrysender.SendIntentOperator(telemetriesgql.EventTypeNetworkPoliciesDeleted, deletedCount)
+		prometheus.IncrementNetpolDeleted(deletedCount)
+	}
+
 	return nil
 }