Skip to content

Commit

Permalink
Add support for creating external traffic policies even if no intents…
Browse files Browse the repository at this point in the history
… point to the same service (#189)
  • Loading branch information
orishoshan committed May 24, 2023
1 parent 1dd7ab8 commit 0828724
Show file tree
Hide file tree
Showing 6 changed files with 332 additions and 45 deletions.
44 changes: 36 additions & 8 deletions src/operator/controllers/external_traffic/endpoints_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,22 @@ const OtterizeExternalNetworkPolicyNameTemplate = "external-access-to-%s"

type EndpointsReconciler struct {
client.Client
Scheme *runtime.Scheme
netpolCreator *NetworkPolicyCreator
Scheme *runtime.Scheme
netpolCreator *NetworkPolicyCreator
createEvenIfNoIntentsFound bool
injectablerecorder.InjectableRecorder
}

func (r *EndpointsReconciler) formatPolicyName(serviceName string) string {
return fmt.Sprintf(OtterizeExternalNetworkPolicyNameTemplate, serviceName)
}

func NewEndpointsReconciler(client client.Client, scheme *runtime.Scheme, enabled bool, enforcementEnabledGlobally bool) *EndpointsReconciler {
func NewEndpointsReconciler(client client.Client, scheme *runtime.Scheme, enabled bool, createEvenIfNoIntentsFound bool, enforcementEnabledGlobally bool) *EndpointsReconciler {
return &EndpointsReconciler{
Client: client,
Scheme: scheme,
netpolCreator: NewNetworkPolicyCreator(client, scheme, enabled, enforcementEnabledGlobally),
Client: client,
Scheme: scheme,
createEvenIfNoIntentsFound: createEvenIfNoIntentsFound,
netpolCreator: NewNetworkPolicyCreator(client, scheme, enabled, createEvenIfNoIntentsFound, enforcementEnabledGlobally),
}
}

Expand Down Expand Up @@ -186,6 +188,15 @@ func (r *EndpointsReconciler) reconcileEndpoints(ctx context.Context, endpoints
}

if len(netpolList.Items) == 0 {
if r.createEvenIfNoIntentsFound {
result, err := r.ReconcileServiceForServiceWithoutIntents(ctx, endpoints, serverLabel, ingressList)
if err != nil {
return ctrl.Result{}, err
}
if !result.IsZero() {
return result, nil
}
}
continue
}

Expand All @@ -200,7 +211,7 @@ func (r *EndpointsReconciler) reconcileEndpoints(ctx context.Context, endpoints

}

if !foundOtterizeNetpolsAffectingPods {
if !foundOtterizeNetpolsAffectingPods && !r.createEvenIfNoIntentsFound {
policyName := r.formatPolicyName(endpoints.Name)
result, err := r.handlePolicyDelete(ctx, policyName, endpoints.Namespace)
if err != nil {
Expand Down Expand Up @@ -241,7 +252,24 @@ func (r *EndpointsReconciler) ReconcileServiceForOtterizeNetpol(ctx context.Cont
return ctrl.Result{}, err
}

err = r.netpolCreator.handleNetworkPolicyCreationOrUpdate(ctx, endpoints, svc, otterizeServiceName, svc, netpol, ingressList, r.formatPolicyName(endpoints.Name))
err = r.netpolCreator.handleNetworkPolicyCreationOrUpdateForNetpol(ctx, endpoints, svc, otterizeServiceName, svc, netpol, ingressList, r.formatPolicyName(endpoints.Name))
if err != nil {
if k8serrors.IsConflict(err) {
return ctrl.Result{Requeue: true}, nil
}
return ctrl.Result{}, err
}
return ctrl.Result{}, nil
}

func (r *EndpointsReconciler) ReconcileServiceForServiceWithoutIntents(ctx context.Context, endpoints *corev1.Endpoints, otterizeServiceName string, ingressList *v1.IngressList) (ctrl.Result, error) {
svc := &corev1.Service{}
err := r.Get(ctx, types.NamespacedName{Name: endpoints.Name, Namespace: endpoints.Namespace}, svc)
if err != nil {
return ctrl.Result{}, err
}

err = r.netpolCreator.handleNetworkPolicyCreationOrUpdateForServiceWithoutIntents(ctx, endpoints, svc, otterizeServiceName, svc, svc, ingressList, r.formatPolicyName(endpoints.Name))
if err != nil {
if k8serrors.IsConflict(err) {
return ctrl.Result{Requeue: true}, nil
Expand Down
24 changes: 17 additions & 7 deletions src/operator/controllers/external_traffic/network_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package external_traffic

import (
"context"
"fmt"
"github.com/otterize/intents-operator/src/operator/api/v1alpha2"
"github.com/otterize/intents-operator/src/shared/injectablerecorder"
"github.com/samber/lo"
Expand Down Expand Up @@ -33,20 +34,29 @@ type NetworkPolicyCreator struct {
client client.Client
scheme *runtime.Scheme
injectablerecorder.InjectableRecorder
enabled bool
enforcementEnabledGlobally bool
enabled bool
createEvenIfNoPreexistingNetworkPolicy bool
enforcementEnabledGlobally bool
}

func NewNetworkPolicyCreator(client client.Client, scheme *runtime.Scheme, enabled bool, enforcementEnabledGlobally bool) *NetworkPolicyCreator {
return &NetworkPolicyCreator{client: client, scheme: scheme, enabled: enabled, enforcementEnabledGlobally: enforcementEnabledGlobally}
func NewNetworkPolicyCreator(client client.Client, scheme *runtime.Scheme, enabled bool, createEvenIfNoPreexistingNetworkPolicy bool, enforcementEnabledGlobally bool) *NetworkPolicyCreator {
return &NetworkPolicyCreator{client: client, scheme: scheme, enabled: enabled, createEvenIfNoPreexistingNetworkPolicy: createEvenIfNoPreexistingNetworkPolicy, enforcementEnabledGlobally: enforcementEnabledGlobally}
}

func (r *NetworkPolicyCreator) handleNetworkPolicyCreationOrUpdateForServiceWithoutIntents(ctx context.Context, endpoints *corev1.Endpoints, owner client.Object, otterizeServiceName string, eventsObject client.Object, svc *corev1.Service, ingressList *v1.IngressList, policyName string) error {
return r.handleNetworkPolicyCreationOrUpdate(ctx, endpoints, owner, otterizeServiceName, eventsObject, metav1.LabelSelector{MatchLabels: svc.Spec.Selector}, ingressList, policyName, fmt.Sprintf("created external traffic network policy for service '%s'", endpoints.GetName()))
}

func (r *NetworkPolicyCreator) handleNetworkPolicyCreationOrUpdateForNetpol(ctx context.Context, endpoints *corev1.Endpoints, owner client.Object, otterizeServiceName string, eventsObject client.Object, netpol *v1.NetworkPolicy, ingressList *v1.IngressList, policyName string) error {
return r.handleNetworkPolicyCreationOrUpdate(ctx, endpoints, owner, otterizeServiceName, eventsObject, netpol.Spec.PodSelector, ingressList, policyName, fmt.Sprintf("created external traffic network policy. service '%s' refers to pods protected by network policy '%s'", endpoints.GetName(), netpol.GetName()))
}

func (r *NetworkPolicyCreator) handleNetworkPolicyCreationOrUpdate(
ctx context.Context, endpoints *corev1.Endpoints, owner client.Object, otterizeServiceName string, eventsObject client.Object, netpol *v1.NetworkPolicy, ingressList *v1.IngressList, policyName string) error {
ctx context.Context, endpoints *corev1.Endpoints, owner client.Object, otterizeServiceName string, eventsObject client.Object, selector metav1.LabelSelector, ingressList *v1.IngressList, policyName string, successMsg string) error {

existingPolicy := &v1.NetworkPolicy{}
errGetExistingPolicy := r.client.Get(ctx, types.NamespacedName{Name: policyName, Namespace: endpoints.GetNamespace()}, existingPolicy)
newPolicy := buildNetworkPolicyObjectForService(endpoints, otterizeServiceName, netpol.Spec.PodSelector, ingressList, policyName)
newPolicy := buildNetworkPolicyObjectForService(endpoints, otterizeServiceName, selector, ingressList, policyName)
err := controllerutil.SetOwnerReference(owner, newPolicy, r.scheme)
if err != nil {
return err
Expand All @@ -64,7 +74,7 @@ func (r *NetworkPolicyCreator) handleNetworkPolicyCreationOrUpdate(
r.RecordWarningEventf(eventsObject, ReasonCreatingExternalTrafficPolicyFailed, "failed to create external traffic network policy: %s", err.Error())
return err
}
r.RecordNormalEventf(eventsObject, ReasonCreatedExternalTrafficPolicy, "created external traffic network policy. service '%s' refers to pods protected by network policy '%s'", endpoints.GetName(), netpol.GetName())
r.RecordNormalEvent(eventsObject, ReasonCreatedExternalTrafficPolicy, successMsg)
}
return nil
} else if errGetExistingPolicy != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) SetupTest() {
s.NetworkPolicyReconciler = NewNetworkPolicyReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, nil, []string{}, true, true)
s.NetworkPolicyReconciler.InjectRecorder(recorder)

s.endpointReconciler = external_traffic.NewEndpointsReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, true, true)
s.endpointReconciler = external_traffic.NewEndpointsReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, true, false, true)
s.endpointReconciler.InjectRecorder(recorder)
err := s.endpointReconciler.InitIngressReferencedServicesIndex(s.Mgr)
s.Require().NoError(err)
Expand Down Expand Up @@ -292,7 +292,7 @@ func (s *ExternalNetworkPolicyReconcilerTestSuite) TestEndpointsReconcilerEnforc
s.AddNodePortService(nodePortServiceName, podIps, podLabels)
s.Require().True(s.Mgr.GetCache().WaitForCacheSync(context.Background()))

endpointReconcilerWithEnforcementDisabled := external_traffic.NewEndpointsReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, true, false)
endpointReconcilerWithEnforcementDisabled := external_traffic.NewEndpointsReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, true, false, false)
recorder := record.NewFakeRecorder(10)
endpointReconcilerWithEnforcementDisabled.InjectRecorder(recorder)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,245 @@
package intents_reconcilers

import (
"context"
"fmt"
otterizev1alpha2 "github.com/otterize/intents-operator/src/operator/api/v1alpha2"
"github.com/otterize/intents-operator/src/operator/controllers/external_traffic"
"github.com/otterize/intents-operator/src/shared/testbase"
"github.com/otterize/intents-operator/src/watcher/reconcilers"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"
v1 "k8s.io/api/networking/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/tools/record"
"path/filepath"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/envtest"
"testing"
)

type ExternalNetworkPolicyReconcilerWithNoIntentsTestSuite struct {
testbase.ControllerManagerTestSuiteBase
IngressReconciler *external_traffic.IngressReconciler
endpointReconciler *external_traffic.EndpointsReconciler
NetworkPolicyReconciler *NetworkPolicyReconciler
podWatcher *reconcilers.PodWatcher
}

func (s *ExternalNetworkPolicyReconcilerWithNoIntentsTestSuite) SetupSuite() {
s.TestEnv = &envtest.Environment{}
var err error
s.TestEnv.CRDDirectoryPaths = []string{filepath.Join("..", "..", "config", "crd")}

s.RestConfig, err = s.TestEnv.Start()
s.Require().NoError(err)
s.Require().NotNil(s.RestConfig)

s.K8sDirectClient, err = kubernetes.NewForConfig(s.RestConfig)
s.Require().NoError(err)
s.Require().NotNil(s.K8sDirectClient)

err = otterizev1alpha2.AddToScheme(s.TestEnv.Scheme)
s.Require().NoError(err)
}

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

recorder := s.Mgr.GetEventRecorderFor("intents-operator")
s.NetworkPolicyReconciler = NewNetworkPolicyReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, nil, []string{}, true, true)
s.NetworkPolicyReconciler.InjectRecorder(recorder)

s.endpointReconciler = external_traffic.NewEndpointsReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, true, true, true)
s.endpointReconciler.InjectRecorder(recorder)
err := s.endpointReconciler.InitIngressReferencedServicesIndex(s.Mgr)
s.Require().NoError(err)

s.IngressReconciler = external_traffic.NewIngressReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, s.endpointReconciler)
s.IngressReconciler.InjectRecorder(recorder)
err = s.IngressReconciler.InitNetworkPoliciesByIngressNameIndex(s.Mgr)
s.Require().NoError(err)

s.podWatcher = reconcilers.NewPodWatcher(s.Mgr.GetClient(), recorder, []string{})
err = s.podWatcher.InitIntentsClientIndices(s.Mgr)
s.Require().NoError(err)
}

// BeforeTest happens AFTER the SetupTest()
func (s *ExternalNetworkPolicyReconcilerWithNoIntentsTestSuite) BeforeTest(_, testName string) {
s.ControllerManagerTestSuiteBase.BeforeTest("", testName)
}

func (s *ExternalNetworkPolicyReconcilerWithNoIntentsTestSuite) TestNetworkPolicyCreateForIngress() {
serviceName := "test-server-ingress-test"

// make sure the network policy was created between the two services based on the intents
np := &v1.NetworkPolicy{}

s.AddDeploymentWithService(serviceName, []string{"1.1.1.1"}, map[string]string{"app": "test"}, nil)
s.Require().True(s.Mgr.GetCache().WaitForCacheSync(context.Background()))

// the ingress reconciler expect the pod watcher labels in order to work
_, err := s.podWatcher.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Namespace: s.TestNamespace, Name: serviceName + "-0"}})
s.Require().NoError(err)

// make sure the ingress network policy doesn't exist yet
externalNetworkPolicyName := fmt.Sprintf(external_traffic.OtterizeExternalNetworkPolicyNameTemplate, serviceName)
err = s.Mgr.GetClient().Get(context.Background(), types.NamespacedName{Namespace: s.TestNamespace, Name: externalNetworkPolicyName}, np)
s.Require().True(errors.IsNotFound(err))

s.AddIngress(serviceName)
s.Require().True(s.Mgr.GetCache().WaitForCacheSync(context.Background()))

res, err := s.IngressReconciler.Reconcile(context.Background(), ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: s.TestNamespace,
Name: serviceName + "-ingress",
},
})

s.Require().NoError(err)
s.Require().Empty(res)

s.Require().True(s.Mgr.GetCache().WaitForCacheSync(context.Background()))
s.WaitUntilCondition(func(assert *assert.Assertions) {
err = s.Mgr.GetClient().Get(context.Background(), types.NamespacedName{Namespace: s.TestNamespace, Name: externalNetworkPolicyName}, np)
assert.NoError(err)
assert.NotEmpty(np)
})
}

func (s *ExternalNetworkPolicyReconcilerWithNoIntentsTestSuite) TestNetworkPolicyCreateForLoadBalancer() {
serviceName := "test-server-load-balancer-test"

// make sure the network policy was created between the two services based on the intents
np := &v1.NetworkPolicy{}

podIps := []string{"1.1.2.1"}
podLabels := map[string]string{"app": "test-load-balancer"}
s.AddDeploymentWithService(serviceName, podIps, podLabels, nil)
s.Require().True(s.Mgr.GetCache().WaitForCacheSync(context.Background()))

// the ingress reconciler expect the pod watcher labels in order to work
_, err := s.podWatcher.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Namespace: s.TestNamespace, Name: serviceName + "-0"}})
s.Require().NoError(err)

// make sure the load balancer network policy doesn't exist yet
loadBalancerServiceName := serviceName + "-lb"
externalNetworkPolicyName := fmt.Sprintf(external_traffic.OtterizeExternalNetworkPolicyNameTemplate, loadBalancerServiceName)
err = s.Mgr.GetClient().Get(context.Background(), types.NamespacedName{Namespace: s.TestNamespace, Name: externalNetworkPolicyName}, np)
s.Require().True(errors.IsNotFound(err))

s.AddLoadBalancerService(loadBalancerServiceName, podIps, podLabels)
s.Require().True(s.Mgr.GetCache().WaitForCacheSync(context.Background()))
res, err := s.endpointReconciler.Reconcile(context.Background(), ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: s.TestNamespace,
Name: loadBalancerServiceName,
},
})

s.Require().NoError(err)
s.Require().Empty(res)

s.Require().True(s.Mgr.GetCache().WaitForCacheSync(context.Background()))
s.WaitUntilCondition(func(assert *assert.Assertions) {
err = s.Mgr.GetClient().Get(context.Background(), types.NamespacedName{Namespace: s.TestNamespace, Name: externalNetworkPolicyName}, np)
assert.NoError(err)
assert.NotEmpty(np)
})
}

func (s *ExternalNetworkPolicyReconcilerWithNoIntentsTestSuite) TestNetworkPolicyCreateForNodePort() {
serviceName := "test-server-node-port-test"

// make sure the network policy was created between the two services based on the intents
np := &v1.NetworkPolicy{}

podIps := []string{"1.1.2.1"}
podLabels := map[string]string{"app": "test-load-balancer"}
s.AddDeploymentWithService(serviceName, podIps, podLabels, nil)
s.Require().True(s.Mgr.GetCache().WaitForCacheSync(context.Background()))

// the ingress reconciler expect the pod watcher labels in order to work
_, err := s.podWatcher.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Namespace: s.TestNamespace, Name: serviceName + "-0"}})
s.Require().NoError(err)

// make sure the load balancer network policy doesn't exist yet
nodePortServiceName := serviceName + "-np"
externalNetworkPolicyName := fmt.Sprintf(external_traffic.OtterizeExternalNetworkPolicyNameTemplate, nodePortServiceName)
err = s.Mgr.GetClient().Get(context.Background(), types.NamespacedName{Namespace: s.TestNamespace, Name: externalNetworkPolicyName}, np)
s.Require().True(errors.IsNotFound(err))

s.AddNodePortService(nodePortServiceName, podIps, podLabels)
s.Require().True(s.Mgr.GetCache().WaitForCacheSync(context.Background()))
res, err := s.endpointReconciler.Reconcile(context.Background(), ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: s.TestNamespace,
Name: nodePortServiceName,
},
})

s.Require().NoError(err)
s.Require().Empty(res)
s.Require().True(s.Mgr.GetCache().WaitForCacheSync(context.Background()))
s.WaitUntilCondition(func(assert *assert.Assertions) {
err = s.Mgr.GetClient().Get(context.Background(), types.NamespacedName{Namespace: s.TestNamespace, Name: externalNetworkPolicyName}, np)
assert.NoError(err)
assert.NotEmpty(np)
})
}

func (s *ExternalNetworkPolicyReconcilerWithNoIntentsTestSuite) TestEndpointsReconcilerEnforcementDisabled() {
serviceName := "test-endpoints-reconciler-enforcement-disabled"

// make sure the network policy was created between the two services based on the intents
np := &v1.NetworkPolicy{}

podIps := []string{"1.1.2.1"}
podLabels := map[string]string{"app": "test-load-balancer"}
s.AddDeploymentWithService(serviceName, podIps, podLabels, nil)
s.Require().True(s.Mgr.GetCache().WaitForCacheSync(context.Background()))

// the ingress reconciler expect the pod watcher labels in order to work
_, err := s.podWatcher.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Namespace: s.TestNamespace, Name: serviceName + "-0"}})
s.Require().NoError(err)

// make sure the load balancer network policy doesn't exist yet
nodePortServiceName := serviceName + "-np"
externalNetworkPolicyName := fmt.Sprintf(external_traffic.OtterizeExternalNetworkPolicyNameTemplate, nodePortServiceName)
err = s.Mgr.GetClient().Get(context.Background(), types.NamespacedName{Namespace: s.TestNamespace, Name: externalNetworkPolicyName}, np)
s.Require().True(errors.IsNotFound(err))

s.AddNodePortService(nodePortServiceName, podIps, podLabels)
s.Require().True(s.Mgr.GetCache().WaitForCacheSync(context.Background()))

endpointReconcilerWithEnforcementDisabled := external_traffic.NewEndpointsReconciler(s.Mgr.GetClient(), s.TestEnv.Scheme, true, true, false)
recorder := record.NewFakeRecorder(10)
endpointReconcilerWithEnforcementDisabled.InjectRecorder(recorder)

res, err := endpointReconcilerWithEnforcementDisabled.Reconcile(context.Background(), ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: s.TestNamespace,
Name: nodePortServiceName,
},
})

s.Require().NoError(err)
s.Require().Empty(res)
s.Require().True(s.Mgr.GetCache().WaitForCacheSync(context.Background()))
err = s.Mgr.GetClient().Get(context.Background(), types.NamespacedName{Namespace: s.TestNamespace, Name: externalNetworkPolicyName}, np)
s.Require().True(errors.IsNotFound(err))
select {
case event := <-recorder.Events:
s.Require().Contains(event, external_traffic.ReasonEnforcementGloballyDisabled)
default:
s.Fail("event not raised")
}
}

func TestExternalNetworkPolicyReconcilerWithNoIntentsTestSuite(t *testing.T) {
suite.Run(t, new(ExternalNetworkPolicyReconcilerWithNoIntentsTestSuite))
}
Loading

0 comments on commit 0828724

Please sign in to comment.