Improvement: Otterize network policies auto-allows DNS traffic if tar…

…get is the cluster's DNS server and an Otterize network policy would block it (#386)

src/operator/controllers/intents_reconcilers/networkpolicy/builders/ingress_dns_server_allow_network_policy.go

@@ -0,0 +1,42 @@
+package builders
+
+import (
+	"context"
+	"github.com/otterize/intents-operator/src/operator/effectivepolicy"
+	"github.com/otterize/intents-operator/src/shared/injectablerecorder"
+	"github.com/samber/lo"
+	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/api/networking/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+	"strings"
+)
+
+type IngressDNSServerAutoAllowNetpolBuilder struct {
+	injectablerecorder.InjectableRecorder
+}
+
+func NewIngressDNSServerAutoAllowNetpolBuilder() *IngressDNSServerAutoAllowNetpolBuilder {
+	return &IngressDNSServerAutoAllowNetpolBuilder{}
+}
+
+// Add UDP allow port 53 if the target is a DNS server in kube-system. This covers 'coredns' and 'kube-dns'.
+func (r *IngressDNSServerAutoAllowNetpolBuilder) buildIngressRulesFromServiceEffectivePolicy(ep effectivepolicy.ServiceEffectivePolicy) []v1.NetworkPolicyIngressRule {
+	ingressRules := make([]v1.NetworkPolicyIngressRule, 0)
+
+	// If the service is not called by any other service, or it's not a DNS server, skip.
+	if len(ep.CalledBy) == 0 ||
+		!strings.HasSuffix(ep.Service.Name, "dns") || ep.Service.Namespace != "kube-system" {
+		return ingressRules
+	}
+	ingressRules = append(ingressRules, v1.NetworkPolicyIngressRule{
+		Ports: []v1.NetworkPolicyPort{{
+			Protocol: lo.ToPtr(corev1.ProtocolUDP),
+			Port:     lo.ToPtr(intstr.FromInt32(53)),
+		}},
+	})
+	return ingressRules
+}
+
+func (r *IngressDNSServerAutoAllowNetpolBuilder) Build(_ context.Context, ep effectivepolicy.ServiceEffectivePolicy) ([]v1.NetworkPolicyIngressRule, error) {
+	return r.buildIngressRulesFromServiceEffectivePolicy(ep), nil
+}

src/operator/controllers/intents_reconcilers/networkpolicy/builders/ingress_dns_server_allow_network_policy_test.go

@@ -0,0 +1,136 @@
+package builders
+
+import (
+	"context"
+	"fmt"
+	otterizev1alpha3 "github.com/otterize/intents-operator/src/operator/api/v1alpha3"
+	"github.com/otterize/intents-operator/src/operator/controllers/intents_reconcilers/consts"
+	"github.com/samber/lo"
+	"github.com/stretchr/testify/suite"
+	"go.uber.org/mock/gomock"
+	v12 "k8s.io/api/core/v1"
+	v1 "k8s.io/api/networking/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/intstr"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"testing"
+)
+
+type DNSServerBuilderNetworkPolicyReconcilerTestSuite struct {
+	RulesBuilderTestSuiteBase
+	Builder *IngressDNSServerAutoAllowNetpolBuilder
+}
+
+func (s *DNSServerBuilderNetworkPolicyReconcilerTestSuite) SetupTest() {
+	s.RulesBuilderTestSuiteBase.SetupTest()
+	s.Builder = NewIngressDNSServerAutoAllowNetpolBuilder()
+	s.Builder.Recorder = s.Recorder
+	s.Reconciler.AddIngressRuleBuilder(s.Builder)
+}
+
+func (s *DNSServerBuilderNetworkPolicyReconcilerTestSuite) TearDownTest() {
+	s.RulesBuilderTestSuiteBase.TearDownTest()
+	s.Builder = nil
+}
+
+func (s *DNSServerBuilderNetworkPolicyReconcilerTestSuite) TestCreateNetworkPolicy() {
+	clientIntentsName := "client-intents"
+	policyName := "coredns-access"
+	serviceName := "test-client"
+	serverNamespace := "kube-system"
+	formattedTargetServer := "coredns-kube-system-dd756f"
+
+	s.Reconciler.EnforcementDefaultState = true
+	namespacedName := types.NamespacedName{
+		Namespace: "kube-system",
+		Name:      clientIntentsName,
+	}
+	req := ctrl.Request{
+		NamespacedName: namespacedName,
+	}
+	intentsSpec := &otterizev1alpha3.IntentsSpec{
+		Service: otterizev1alpha3.Service{Name: serviceName},
+		Calls: []otterizev1alpha3.Intent{
+			{
+				Name: fmt.Sprintf("coredns.%s", serverNamespace),
+			},
+		},
+	}
+
+	s.expectGetAllEffectivePolicies([]otterizev1alpha3.ClientIntents{{Spec: intentsSpec, ObjectMeta: metav1.ObjectMeta{Name: namespacedName.Name, Namespace: namespacedName.Namespace}}})
+
+	// Search for existing NetworkPolicy
+	emptyNetworkPolicy := &v1.NetworkPolicy{}
+	networkPolicyNamespacedName := types.NamespacedName{
+		Namespace: serverNamespace,
+		Name:      policyName,
+	}
+	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 {
+			return apierrors.NewNotFound(v1.Resource("networkpolicy"), name.Name)
+		})
+
+	// Create NetworkPolicy
+	newPolicy := ingressDNSnetworkPolicyIngressTemplate(
+		policyName,
+		serverNamespace,
+		formattedTargetServer,
+		testNamespace,
+	)
+	s.Client.EXPECT().Create(gomock.Any(), gomock.Eq(newPolicy)).Return(nil)
+
+	selector := labels.SelectorFromSet(labels.Set(map[string]string{
+		otterizev1alpha3.OtterizeServiceLabelKey: formattedTargetServer,
+	}))
+
+	s.externalNetpolHandler.EXPECT().HandlePodsByLabelSelector(gomock.Any(), serverNamespace, selector)
+	s.ignoreRemoveOrphan()
+
+	res, err := s.EPIntentsReconciler.Reconcile(context.Background(), req)
+	s.NoError(err)
+	s.Empty(res)
+	s.ExpectEvent(consts.ReasonCreatedNetworkPolicies)
+}
+
+func ingressDNSnetworkPolicyIngressTemplate(
+	policyName string,
+	targetNamespace string,
+	formattedTargetServer string,
+	intentsObjNamespaces ...string,
+) *v1.NetworkPolicy {
+	ingressRules := lo.Map(intentsObjNamespaces, func(namespace string, _ int) v1.NetworkPolicyIngressRule {
+		return v1.NetworkPolicyIngressRule{
+			Ports: []v1.NetworkPolicyPort{{
+				Protocol: lo.ToPtr(v12.ProtocolUDP),
+				Port:     &intstr.IntOrString{Type: intstr.Int, IntVal: 53},
+			}},
+		}
+	})
+	return &v1.NetworkPolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      policyName,
+			Namespace: targetNamespace,
+			Labels: map[string]string{
+				otterizev1alpha3.OtterizeNetworkPolicy: formattedTargetServer,
+			},
+		},
+		Spec: v1.NetworkPolicySpec{
+			PolicyTypes: []v1.PolicyType{v1.PolicyTypeIngress},
+			PodSelector: metav1.LabelSelector{
+				MatchLabels: map[string]string{
+					otterizev1alpha3.OtterizeServiceLabelKey: formattedTargetServer,
+				},
+			},
+			Ingress: ingressRules,
+			Egress:  make([]v1.NetworkPolicyEgressRule, 0),
+		},
+	}
+}
+
+func TestDNSServerBuilderNetworkPolicyReconcilerTestSuite(t *testing.T) {
+	suite.Run(t, new(DNSServerBuilderNetworkPolicyReconcilerTestSuite))
+}

src/operator/main.go

@@ -216,7 +216,9 @@ func main() {
 
 	additionalIntentsReconcilers := make([]reconcilergroup.ReconcilerWithEvents, 0)
 	svcNetworkPolicyBuilder := builders.NewPortNetworkPolicyReconciler(mgr.GetClient())
-	epNetpolReconciler := networkpolicy.NewReconciler(mgr.GetClient(), scheme, extNetpolHandler, watchedNamespaces, enforcementConfig.EnableNetworkPolicy, enforcementConfig.EnforcementDefaultState, []networkpolicy.IngressRuleBuilder{ingressRulesBuilder, svcNetworkPolicyBuilder}, make([]networkpolicy.EgressRuleBuilder, 0))
+	dnsServerNetpolBuilder := builders.NewIngressDNSServerAutoAllowNetpolBuilder()
+	epNetpolReconciler := networkpolicy.NewReconciler(mgr.GetClient(), scheme, extNetpolHandler, watchedNamespaces, enforcementConfig.EnableNetworkPolicy, enforcementConfig.EnforcementDefaultState,
+		[]networkpolicy.IngressRuleBuilder{ingressRulesBuilder, svcNetworkPolicyBuilder, dnsServerNetpolBuilder}, make([]networkpolicy.EgressRuleBuilder, 0))
 	epGroupReconciler := effectivepolicy.NewGroupReconciler(mgr.GetClient(), scheme, epNetpolReconciler)
 	if enforcementConfig.EnableEgressNetworkPolicyReconcilers {
 		egressNetworkPolicyHandler := builders.NewEgressNetworkPolicyBuilder()