Skip to content

Commit

Permalink
Revert "Introduce a new type: serviceEffectivePolicy as a better ab…
Browse files Browse the repository at this point in the history 
…straction than ClientIntents for reconciliation. Implement `EffectivePolicyReconciler` for ingress network policies. (#323)"

This reverts commit 4da0664.
  • Loading branch information
@orishoshan
orishoshan committed Jan 20, 2024
1 parent f38bc95 commit 06c814a
Show file tree
Hide file tree
Showing 17 changed files with 1,171 additions and 607 deletions.
2 changes: 1 addition & 1 deletion helm-charts
Submodule helm-charts updated 4 files
+2 −2 .github/workflows/bump-version.yaml
+2 −2 credentials-operator/Chart.yaml
+2 −2 intents-operator/Chart.yaml
+1 −1 otterize-kubernetes/Chart.yaml
28 changes: 23 additions & 5 deletions src/operator/controllers/intents_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,14 @@ import (
"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/ingress_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"
"github.com/otterize/intents-operator/src/shared/initonce"
"github.com/otterize/intents-operator/src/shared/operator_cloud_client"
"github.com/otterize/intents-operator/src/shared/reconcilergroup"
"github.com/otterize/intents-operator/src/shared/serviceidresolver"
Expand Down Expand Up @@ -68,14 +70,17 @@ type EnforcementConfig struct {

// IntentsReconciler reconciles a Intents object
type IntentsReconciler struct {
group *reconcilergroup.Group
client client.Client
group *reconcilergroup.Group
client client.Client
initOnce initonce.InitOnce
networkPolicyReconciler *ingress_network_policy.NetworkPolicyReconciler
}

func NewIntentsReconciler(
client client.Client,
scheme *runtime.Scheme,
kafkaServerStore kafkaacls.ServersStore,
networkPolicyReconciler *ingress_network_policy.NetworkPolicyReconciler,
portNetpolReconciler *port_network_policy.PortNetworkPolicyReconciler,
egressNetpolReconciler *egress_network_policy.EgressNetworkPolicyReconciler,
portEgressNetpolReconciler *port_egress_network_policy.PortEgressNetworkPolicyReconciler,
Expand All @@ -93,6 +98,7 @@ func NewIntentsReconciler(
intents_reconcilers.NewPodLabelReconciler(client, scheme),
intents_reconcilers.NewKafkaACLReconciler(client, scheme, kafkaServerStore, enforcementConfig.EnableKafkaACL, kafkaacls.NewKafkaIntentsAdmin, enforcementConfig.EnforcementDefaultState, operatorPodName, operatorPodNamespace, serviceIdResolver),
intents_reconcilers.NewIstioPolicyReconciler(client, scheme, restrictToNamespaces, enforcementConfig.EnableIstioPolicy, enforcementConfig.EnforcementDefaultState),
networkPolicyReconciler,
}
reconcilers = append(reconcilers, additionalReconcilers...)
reconcilersGroup := reconcilergroup.NewGroup(
Expand All @@ -108,8 +114,9 @@ func NewIntentsReconciler(
reconcilersGroup.AddToGroup(portNetpolReconciler)

intentsReconciler := &IntentsReconciler{
group: reconcilersGroup,
client: client,
group: reconcilersGroup,
client: client,
networkPolicyReconciler: networkPolicyReconciler,
}

if telemetriesconfig.IsUsageTelemetryEnabled() {
Expand Down Expand Up @@ -148,9 +155,16 @@ func NewIntentsReconciler(
// Reconcile is part of the main kubernetes reconciliation loop which aims to
// move the current state of the cluster closer to the desired state.
func (r *IntentsReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
err := r.initOnce.Do(func() error {
return r.intentsReconcilerInit(ctx)
})
if err != nil {
return ctrl.Result{}, errors.Wrap(err)
}

intents := &otterizev1alpha3.ClientIntents{}

err := r.client.Get(ctx, req.NamespacedName, intents)
err = r.client.Get(ctx, req.NamespacedName, intents)
if err != nil {
if k8serrors.IsNotFound(err) {
return ctrl.Result{}, nil
Expand Down Expand Up @@ -186,6 +200,10 @@ func (r *IntentsReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
return result, nil
}

func (r *IntentsReconciler) intentsReconcilerInit(ctx context.Context) error {
return r.networkPolicyReconciler.CleanAllNamespaces(ctx)
}

// SetupWithManager sets up the controller with the Manager.
func (r *IntentsReconciler) SetupWithManager(mgr ctrl.Manager) error {
err := ctrl.NewControllerManagedBy(mgr).
Expand Down
1 change: 1 addition & 0 deletions src/operator/controllers/intents_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ func (s *IntentsControllerTestSuite) SetupTest() {
nil,
nil,
nil,
nil,
EnforcementConfig{},
nil,
"",
Expand Down
1 change: 0 additions & 1 deletion src/operator/controllers/intents_reconcilers/consts/consts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ const (
ReasonNetworkPolicyCreationDisabled = "NetworkPolicyCreationDisabled"
ReasonGettingNetworkPolicyFailed = "GettingNetworkPolicyFailed"
ReasonRemovingNetworkPolicyFailed = "RemovingNetworkPolicyFailed"
ReasonReconcilingNetworkPolicyFailed = "ReconcilingNetworkPolicyFailed"
ReasonNamespaceNotAllowed = "NamespaceNotAllowed"
ReasonCreatingNetworkPoliciesFailed = "CreatingNetworkPoliciesFailed"
ReasonCreatedNetworkPolicies = "CreatedNetworkPolicies"
Expand Down
36 changes: 16 additions & 20 deletions ...tents_reconcilers/external_traffic_network_policy/external_traffic_network_policy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,8 @@ import (
otterizev1alpha3 "github.com/otterize/intents-operator/src/operator/api/v1alpha3"
"github.com/otterize/intents-operator/src/operator/controllers"
"github.com/otterize/intents-operator/src/operator/controllers/external_traffic"
"github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers"
"github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers/ingress_network_policy"
"github.com/otterize/intents-operator/src/operator/controllers/pod_reconcilers"
"github.com/otterize/intents-operator/src/operator/effectivepolicy"
"github.com/otterize/intents-operator/src/shared/operatorconfig/allowexternaltraffic"
"github.com/otterize/intents-operator/src/shared/testbase"
"github.com/stretchr/testify/assert"
Expand All @@ -33,10 +31,10 @@ import (

type ExternalNetworkPolicyReconcilerTestSuite struct {
testbase.ControllerManagerTestSuiteBase
IngressReconciler *external_traffic.IngressReconciler
endpointReconciler external_traffic.EndpointsReconciler
EffectivePolicyIntentsReconciler *intents_reconcilers.ServiceEffectivePolicyIntentsReconciler
podWatcher *pod_reconcilers.PodWatcher
IngressReconciler *external_traffic.IngressReconciler
endpointReconciler external_traffic.EndpointsReconciler
NetworkPolicyReconciler *ingress_network_policy.NetworkPolicyReconciler
podWatcher *pod_reconcilers.PodWatcher
}

func (s *ExternalNetworkPolicyReconcilerTestSuite) SetupSuite() {
Expand Down Expand Up @@ -64,11 +62,9 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) SetupTest() {

recorder := s.Mgr.GetEventRecorderFor("intents-operator")
netpolHandler := external_traffic.NewNetworkPolicyHandler(s.Mgr.GetClient(), s.TestEnv.Scheme, allowexternaltraffic.IfBlockedByOtterize)
epNetpolReconciler := ingress_network_policy.NewIngressNetpolEffectivePolicyReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, netpolHandler, []string{}, true, true, allowexternaltraffic.IfBlockedByOtterize)
epReconciler := effectivepolicy.NewGroupReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, epNetpolReconciler)
s.EffectivePolicyIntentsReconciler = intents_reconcilers.NewServiceEffectiveIntentsReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, epReconciler)
s.NetworkPolicyReconciler = ingress_network_policy.NewNetworkPolicyReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, netpolHandler, []string{}, true, true, allowexternaltraffic.IfBlockedByOtterize)
s.Require().NoError((&controllers.IntentsReconciler{}).InitIntentsServerIndices(s.Mgr))
s.EffectivePolicyIntentsReconciler.InjectRecorder(recorder)
s.NetworkPolicyReconciler.InjectRecorder(recorder)

s.endpointReconciler = external_traffic.NewEndpointsReconciler(s.Mgr.GetClient(), netpolHandler)
s.endpointReconciler.InjectRecorder(recorder)
Expand Down Expand Up @@ -96,7 +92,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForIng
})
s.Require().NoError(err)

res, err := s.EffectivePolicyIntentsReconciler.Reconcile(context.Background(), ctrl.Request{
res, err := s.NetworkPolicyReconciler.Reconcile(context.Background(), ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: s.TestNamespace,
Name: intents.Name,
Expand Down Expand Up @@ -150,7 +146,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForLoa
})
s.Require().NoError(err)

res, err := s.EffectivePolicyIntentsReconciler.Reconcile(context.Background(), ctrl.Request{
res, err := s.NetworkPolicyReconciler.Reconcile(context.Background(), ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: s.TestNamespace,
Name: intents.Name,
Expand Down Expand Up @@ -206,7 +202,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForLoa
})
s.Require().NoError(err)

res, err := s.EffectivePolicyIntentsReconciler.Reconcile(context.Background(), ctrl.Request{
res, err := s.NetworkPolicyReconciler.Reconcile(context.Background(), ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: s.TestNamespace,
Name: intents.Name,
Expand Down Expand Up @@ -263,7 +259,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForLoa
})

s.WaitUntilCondition(func(assert *assert.Assertions) {
res, err = s.EffectivePolicyIntentsReconciler.Reconcile(context.Background(), ctrl.Request{
res, err = s.NetworkPolicyReconciler.Reconcile(context.Background(), ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: s.TestNamespace,
Name: intents.Name,
Expand All @@ -276,7 +272,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForLoa
})

s.WaitUntilCondition(func(assert *assert.Assertions) {
res, err = s.EffectivePolicyIntentsReconciler.Reconcile(context.Background(), ctrl.Request{
res, err = s.NetworkPolicyReconciler.Reconcile(context.Background(), ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: s.TestNamespace,
Name: intents.Name,
Expand Down Expand Up @@ -304,7 +300,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForLoa
}})
s.Require().NoError(err)

res, err := s.EffectivePolicyIntentsReconciler.Reconcile(context.Background(), ctrl.Request{
res, err := s.NetworkPolicyReconciler.Reconcile(context.Background(), ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: s.TestNamespace,
Name: intents.Name,
Expand All @@ -313,7 +309,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForLoa
s.Require().NoError(err)
s.Require().Empty(res)

res2, err := s.EffectivePolicyIntentsReconciler.Reconcile(context.Background(), ctrl.Request{
res2, err := s.NetworkPolicyReconciler.Reconcile(context.Background(), ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: secondaryNamespace,
Name: secondIntents.Name,
Expand Down Expand Up @@ -369,7 +365,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForLoa
assert.NotNil(intentsDeleted.DeletionTimestamp)
})

res, err = s.EffectivePolicyIntentsReconciler.Reconcile(context.Background(), ctrl.Request{
res, err = s.NetworkPolicyReconciler.Reconcile(context.Background(), ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: s.TestNamespace,
Name: intents.Name,
Expand Down Expand Up @@ -398,7 +394,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) TestNetworkPolicyCreateForNod
})
s.Require().NoError(err)

res, err := s.EffectivePolicyIntentsReconciler.Reconcile(context.Background(), ctrl.Request{
res, err := s.NetworkPolicyReconciler.Reconcile(context.Background(), ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: s.TestNamespace,
Name: intents.Name,
Expand Down Expand Up @@ -453,7 +449,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) TestEndpointsReconcilerNetwor
})
s.Require().NoError(err)

res, err := s.EffectivePolicyIntentsReconciler.Reconcile(context.Background(), ctrl.Request{
res, err := s.NetworkPolicyReconciler.Reconcile(context.Background(), ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: s.TestNamespace,
Name: intents.Name,
Expand Down
21 changes: 8 additions & 13 deletions ...s/external_traffic_network_policy/external_traffic_network_policy_with_no_intents_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@ import (
otterizev1alpha3 "github.com/otterize/intents-operator/src/operator/api/v1alpha3"
"github.com/otterize/intents-operator/src/operator/controllers"
"github.com/otterize/intents-operator/src/operator/controllers/external_traffic"
"github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers"
"github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers/ingress_network_policy"
"github.com/otterize/intents-operator/src/operator/controllers/pod_reconcilers"
"github.com/otterize/intents-operator/src/operator/effectivepolicy"
"github.com/otterize/intents-operator/src/shared/operatorconfig/allowexternaltraffic"
"github.com/otterize/intents-operator/src/shared/testbase"
"github.com/stretchr/testify/assert"
Expand All @@ -32,10 +30,10 @@ import (

type ExternalNetworkPolicyReconcilerWithNoIntentsTestSuite struct {
testbase.ControllerManagerTestSuiteBase
IngressReconciler *external_traffic.IngressReconciler
endpointReconciler external_traffic.EndpointsReconciler
EffectivePolicyIntentsReconciler *intents_reconcilers.ServiceEffectivePolicyIntentsReconciler
podWatcher *pod_reconcilers.PodWatcher
IngressReconciler *external_traffic.IngressReconciler
endpointReconciler external_traffic.EndpointsReconciler
NetworkPolicyReconciler *ingress_network_policy.NetworkPolicyReconciler
podWatcher *pod_reconcilers.PodWatcher
}

func (s *ExternalNetworkPolicyReconcilerWithNoIntentsTestSuite) SetupSuite() {
Expand All @@ -55,19 +53,16 @@ func (s *ExternalNetworkPolicyReconcilerWithNoIntentsTestSuite) SetupSuite() {
utilruntime.Must(clientgoscheme.AddToScheme(s.TestEnv.Scheme))
utilruntime.Must(istiosecurityscheme.AddToScheme(s.TestEnv.Scheme))
utilruntime.Must(otterizev1alpha2.AddToScheme(s.TestEnv.Scheme))
utilruntime.Must(otterizev1alpha3.AddToScheme(s.TestEnv.Scheme))
}

func (s *ExternalNetworkPolicyReconcilerWithNoIntentsTestSuite) SetupTest() {
s.ControllerManagerTestSuiteBase.SetupTest()

recorder := s.Mgr.GetEventRecorderFor("intents-operator")
netpolHandler := external_traffic.NewNetworkPolicyHandler(s.Mgr.GetClient(), s.TestEnv.Scheme, allowexternaltraffic.Always)
netpolApplier := ingress_network_policy.NewIngressNetpolEffectivePolicyReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, netpolHandler, []string{}, true, true, allowexternaltraffic.Always)
groupReconciler := effectivepolicy.NewGroupReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, netpolApplier)
s.EffectivePolicyIntentsReconciler = intents_reconcilers.NewServiceEffectiveIntentsReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, groupReconciler)
s.NetworkPolicyReconciler = ingress_network_policy.NewNetworkPolicyReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, netpolHandler, []string{}, true, true, allowexternaltraffic.Always)
s.Require().NoError((&controllers.IntentsReconciler{}).InitIntentsServerIndices(s.Mgr))
s.EffectivePolicyIntentsReconciler.InjectRecorder(recorder)
s.NetworkPolicyReconciler.InjectRecorder(recorder)

s.endpointReconciler = external_traffic.NewEndpointsReconciler(s.Mgr.GetClient(), netpolHandler)
s.endpointReconciler.InjectRecorder(recorder)
Expand Down Expand Up @@ -247,7 +242,7 @@ func (s *ExternalNetworkPolicyReconcilerWithNoIntentsTestSuite) TestNetworkPolic
})
s.Require().NoError(err)

res, err := s.EffectivePolicyIntentsReconciler.Reconcile(context.Background(), ctrl.Request{
res, err := s.NetworkPolicyReconciler.Reconcile(context.Background(), ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: s.TestNamespace,
Name: intents.Name,
Expand Down Expand Up @@ -303,7 +298,7 @@ func (s *ExternalNetworkPolicyReconcilerWithNoIntentsTestSuite) TestNetworkPolic
assert.NotNil(intentsDeleted.DeletionTimestamp)
})

res, err = s.EffectivePolicyIntentsReconciler.Reconcile(context.Background(), ctrl.Request{
res, err = s.NetworkPolicyReconciler.Reconcile(context.Background(), ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: s.TestNamespace,
Name: intents.Name,
Expand Down
Loading

0 comments on commit 06c814a

Please sign in to comment.