Skip to content

Commit

Permalink
Fix a rare bug where external network policies were not deleted due t…
Browse files Browse the repository at this point in the history
…o a race condition when the enforcement was turned off (#496)
  • Loading branch information
omris94 authored Sep 29, 2024
1 parent fae6442 commit 53935de
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package external_traffic

import (
"context"
"github.com/otterize/intents-operator/src/shared/errors"
"github.com/otterize/intents-operator/src/shared/injectablerecorder"
"github.com/samber/lo"
"github.com/sirupsen/logrus"
v1 "k8s.io/api/networking/v1"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
)

//+kubebuilder:rbac:groups="networking.k8s.io",resources=networkpolicies,verbs=get;update;patch;list;watch;delete;create

type NetworkPolicyReconciler struct {
client.Client
extNetpolHandler *NetworkPolicyHandler
injectablerecorder.InjectableRecorder
}

func NewNetworkPolicyReconciler(
client client.Client,
extNetpolHandler *NetworkPolicyHandler,
) *NetworkPolicyReconciler {
return &NetworkPolicyReconciler{
Client: client,
extNetpolHandler: extNetpolHandler,
}
}

func (r *NetworkPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error {
recorder := mgr.GetEventRecorderFor("intents-operator")
r.InjectRecorder(recorder)

return ctrl.NewControllerManagedBy(mgr).
For(&v1.NetworkPolicy{}).
WithOptions(controller.Options{RecoverPanic: lo.ToPtr(true)}).
Complete(r)
}

// Reconcile handles network policy creation, update and delete. In all of these cases, it calls the NetworkPolicyHandler
// to handle the pods in the namespace of the network policy.
func (r *NetworkPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
logrus.Info("Handling external for NetworkPolicy for namespace: ", req.Namespace)
err := r.extNetpolHandler.HandlePodsByNamespace(ctx, req.Namespace)
if err != nil {
return ctrl.Result{}, errors.Wrap(err)
}

return ctrl.Result{}, nil
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ func (s *RulesBuilderTestSuiteBase) SetupTest() {

epReconciler.InjectableRecorder.Recorder = s.Recorder
s.EPIntentsReconciler.Recorder = s.Recorder
s.externalNetpolHandler.EXPECT().HandleAllPods(gomock.Any()).AnyTimes()
}

func (s *RulesBuilderTestSuiteBase) TearDownTest() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type IngressRuleBuilder interface {
type ExternalNetpolHandler interface {
HandlePodsByLabelSelector(ctx context.Context, namespace string, labelSelector labels.Selector) error
HandleBeforeAccessPolicyRemoval(ctx context.Context, accessPolicy *v1.NetworkPolicy) error
HandleAllPods(ctx context.Context) error
}

type Reconciler struct {
Expand Down Expand Up @@ -127,6 +128,13 @@ func (r *Reconciler) ReconcileEffectivePolicies(ctx context.Context, eps []effec
if currentPolicies.Len() != 0 {
telemetrysender.SendIntentOperator(telemetriesgql.EventTypeNetworkPoliciesCreated, currentPolicies.Len())
}

// We do it to make sure any excess external allow policies are removed
err = r.extNetpolHandler.HandleAllPods(ctx)
if err != nil {
return currentPolicies.Len(), []error{errors.Wrap(err)}
}

return currentPolicies.Len(), nil
}

Expand Down
5 changes: 5 additions & 0 deletions src/operator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ func main() {

externalPolicySvcReconciler := external_traffic.NewServiceReconciler(mgr.GetClient(), extNetpolHandler)
ingressReconciler := external_traffic.NewIngressReconciler(mgr.GetClient(), extNetpolHandler)
netpolReconciler := external_traffic.NewNetworkPolicyReconciler(mgr.GetClient(), extNetpolHandler)

if !enforcementConfig.EnforcementDefaultState {
logrus.Infof("Running with enforcement disabled globally, won't perform any enforcement")
Expand Down Expand Up @@ -424,6 +425,10 @@ func main() {
logrus.WithError(err).Panic("unable to create controller", "controller", "Ingress")
}

if err = netpolReconciler.SetupWithManager(mgr); err != nil {
logrus.WithError(err).Panic("unable to create controller", "controller", "NetworkPolicy")
}

kafkaServerConfigReconciler := controllers.NewKafkaServerConfigReconciler(
mgr.GetClient(),
mgr.GetScheme(),
Expand Down
2 changes: 1 addition & 1 deletion src/shared/operator_cloud_client/cloud_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func (c *CloudClientImpl) ReportExternallyAccessibleServices(ctx context.Context
func (c *CloudClientImpl) ReportProtectedServices(ctx context.Context, namespace string, protectedServices []graphqlclient.ProtectedServiceInput) error {
logrus.WithField("namespace", namespace).
WithField("count", len(protectedServices)).
Infof("Reporting network policies")
Info("Reporting protected services")

_, err := graphqlclient.ReportProtectedServicesSnapshot(ctx, c.client, namespace, protectedServices)
return errors.Wrap(err)
Expand Down

0 comments on commit 53935de

Please sign in to comment.