NE-1476 - Added network policies for DNS

knobunc · knobunc · commit 834a6e1583a9 · 2025-08-11T11:34:30.000-04:00
Added the framework for network policies for DNS for the operator and the dns pods. The operator has a deny all network policy that for the openshift-dns-operator namespace and an allow policy for egress to the apiserver and dns ports at any IP. The operator installs a deny all network policy for the openshift-dns namespace. Then for each dns that it manages it installs an allow policy for ingress for dns traffic and metrics. It has to allow ingress from the dns pods to any IP because we allow configuration to set the upstream server and port, so any valid IP and port needs to be allowed. It also needs access to the api server, but that is covered by the wildcard allow policy. https://issues.redhat.com/browse/NE-1476
diff --git a/hack/release-local.sh b/hack/release-local.sh
@@ -29,9 +29,9 @@ if [[ "${TEMP_COMMIT}" == "true" ]]; then
 fi
 
 cp -R manifests/* $MANIFESTS
-cat manifests/0000_70_dns-operator_02-deployment.yaml | sed "s~openshift/origin-cluster-dns-operator:latest~$REPO:$REV~" > "$MANIFESTS/0000_70_dns-operator_02-deployment.yaml"
+cat manifests/0000_70_dns-operator_03-deployment.yaml | sed "s~openshift/origin-cluster-dns-operator:latest~$REPO:$REV~" > "$MANIFESTS/0000_70_dns-operator_03-deployment.yaml"
 # To simulate CVO, ClusterOperator resource need to be created by the operator.
-rm $MANIFESTS/0000_70_dns-operator_03-cluster-operator.yaml
+rm $MANIFESTS/0000_70_dns-operator_04-cluster-operator.yaml
 
 echo "Pushed $REPO:$REV"
 echo "Install manifests using:"
diff --git a/manifests/0000_70_dns-operator_00-cluster-role.yaml b/manifests/0000_70_dns-operator_00-cluster-role.yaml
@@ -139,3 +139,11 @@ rules:
   - get
   - update
   - patch
+
+- apiGroups:
+  - networking.k8s.io
+  resources:
+  - networkpolicies
+  verbs:
+  - "*"
+  
diff --git a/manifests/0000_70_dns-operator_01-role.yaml b/manifests/0000_70_dns-operator_01-role.yaml
@@ -27,3 +27,10 @@ rules:
   - services
   verbs:
   - "*"
+
+- apiGroups:
+  - networking.k8s.io
+  resources:
+  - networkpolicies
+  verbs:
+  - "*"
diff --git a/manifests/0000_70_dns-operator_02-network-policy.yaml b/manifests/0000_70_dns-operator_02-network-policy.yaml
@@ -0,0 +1,44 @@
+# We control the namespace, deny anything we do not explicitly allow
+apiVersion: networking.k8s.io/v1
+kind: NetworkPolicy
+metadata:
+  name: dns-operator-deny-all
+  namespace: openshift-dns-operator
+  annotations:
+    include.release.openshift.io/self-managed-high-availability: "true"
+    include.release.openshift.io/single-node-developer: "true"
+spec:
+  podSelector: {}
+  policyTypes:
+  - Ingress
+  - Egress
+---
+### Allow the operators to talk to the apiserver and dns
+### Allow access to the metrics ports on the operators
+apiVersion: networking.k8s.io/v1
+kind: NetworkPolicy
+metadata:
+  name: dns-operator-allow
+  namespace: openshift-dns-operator
+  annotations:
+    include.release.openshift.io/self-managed-high-availability: "true"
+    include.release.openshift.io/single-node-developer: "true"
+spec:
+  podSelector:
+    matchLabels:
+      name: dns-operator
+  policyTypes:
+  - Egress
+  - Ingress
+  egress:
+  - ports:
+    - protocol: TCP
+      port: 6443
+    - protocol: TCP
+      port: 53
+    - protocol: UDP
+      port: 53
+  ingress:
+  - ports:
+    - protocol: TCP
+      port: 9393
diff --git a/manifests/0000_70_dns-operator_03-deployment-ibm-cloud-managed.yaml b/manifests/0000_70_dns-operator_03-deployment-ibm-cloud-managed.yaml
diff --git a/manifests/0000_70_dns-operator_03-deployment.yaml b/manifests/0000_70_dns-operator_03-deployment.yaml
diff --git a/manifests/0000_70_dns-operator_04-cluster-operator.yaml b/manifests/0000_70_dns-operator_04-cluster-operator.yaml
diff --git a/pkg/manifests/assets/dns/networkpolicy-allow.yaml b/pkg/manifests/assets/dns/networkpolicy-allow.yaml
@@ -0,0 +1,29 @@
+# DNS Pods
+#  Egress to api server and upstream dns (can be wildcarded, so allow any egress)
+#  Ingress to dns on port 5353 (TCP and UDP) and to metrics (9154)
+apiVersion: networking.k8s.io/v1
+kind: NetworkPolicy
+# name, namespace,labels and annotations are set at runtime
+spec:
+  podSelector:
+    # matchLabels are set at runtime
+    matchLabels: {}
+  ingress:
+  - ports:
+    - protocol: TCP
+      port: 9154
+    - protocol: UDP
+      port: 5353
+    - protocol: TCP
+      port: 5353
+    - protocol: TCP
+      port: 8080
+    - protocol: TCP
+      port: 8181
+  egress:
+  - to:
+    - ipBlock:
+        cidr: 0.0.0.0/0
+  policyTypes:
+  - Ingress
+  - Egress
diff --git a/pkg/manifests/assets/networkpolicy-deny-all.yaml b/pkg/manifests/assets/networkpolicy-deny-all.yaml
@@ -0,0 +1,11 @@
+# Default deny all policy for all pods in the namespace
+apiVersion: networking.k8s.io/v1
+kind: NetworkPolicy
+metadata:
+    name: openshift-dns-deny-all
+    namespace: openshift-dns
+spec:
+ podSelector: {}
+ policyTypes:
+ - Ingress
+ - Egress
diff --git a/pkg/manifests/manifests.go b/pkg/manifests/manifests.go
@@ -7,18 +7,22 @@ import (
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	networkingv1 "k8s.io/api/networking/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 
 	"k8s.io/apimachinery/pkg/util/yaml"
 )
 
 const (
+	NetworkPolicyDenyAllAsset = "assets/networkpolicy-deny-all.yaml"
+
 	DNSNamespaceAsset          = "assets/dns/namespace.yaml"
 	DNSServiceAccountAsset     = "assets/dns/service-account.yaml"
 	DNSClusterRoleAsset        = "assets/dns/cluster-role.yaml"
 	DNSClusterRoleBindingAsset = "assets/dns/cluster-role-binding.yaml"
 	DNSDaemonSetAsset          = "assets/dns/daemonset.yaml"
 	DNSServiceAsset            = "assets/dns/service.yaml"
+	DNSNetworkPolicyAsset      = "assets/dns/networkpolicy-allow.yaml"
 
 	MetricsClusterRoleAsset        = "assets/dns/metrics/cluster-role.yaml"
 	MetricsClusterRoleBindingAsset = "assets/dns/metrics/cluster-role-binding.yaml"
@@ -55,6 +59,14 @@ func MustAssetReader(asset string) io.Reader {
 	return bytes.NewReader(MustAsset(asset))
 }
 
+func NetworkPolicyDenyAll() *networkingv1.NetworkPolicy {
+	np, err := NewNetworkPolicy(MustAssetReader(NetworkPolicyDenyAllAsset))
+	if err != nil {
+		panic(err)
+	}
+	return np
+}
+
 func DNSNamespace() *corev1.Namespace {
 	ns, err := NewNamespace(MustAssetReader(DNSNamespaceAsset))
 	if err != nil {
@@ -103,6 +115,14 @@ func DNSService() *corev1.Service {
 	return s
 }
 
+func DNSNetworkPolicy() *networkingv1.NetworkPolicy {
+	np, err := NewNetworkPolicy(MustAssetReader(DNSNetworkPolicyAsset))
+	if err != nil {
+		panic(err)
+	}
+	return np
+}
+
 func MetricsClusterRole() *rbacv1.ClusterRole {
 	cr, err := NewClusterRole(MustAssetReader(MetricsClusterRoleAsset))
 	if err != nil {
@@ -220,3 +240,11 @@ func NewNamespace(manifest io.Reader) (*corev1.Namespace, error) {
 	}
 	return &ns, nil
 }
+
+func NewNetworkPolicy(manifest io.Reader) (*networkingv1.NetworkPolicy, error) {
+	np := networkingv1.NetworkPolicy{}
+	if err := yaml.NewYAMLOrJSONDecoder(manifest, 100).Decode(&np); err != nil {
+		return nil, err
+	}
+	return &np, nil
+}
diff --git a/pkg/manifests/manifests_test.go b/pkg/manifests/manifests_test.go
@@ -5,12 +5,15 @@ import (
 )
 
 func TestManifests(t *testing.T) {
+	NetworkPolicyDenyAll()
+
 	DNSServiceAccount()
 	DNSClusterRole()
 	DNSClusterRoleBinding()
 	DNSNamespace()
 	DNSDaemonSet()
 	DNSService()
+	DNSNetworkPolicy()
 
 	MetricsClusterRole()
 	MetricsClusterRoleBinding()
diff --git a/pkg/operator/controller/controller.go b/pkg/operator/controller/controller.go
@@ -18,6 +18,7 @@ import (
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	networkingv1 "k8s.io/api/networking/v1"
 
 	"github.com/apparentlymart/go-cidr/cidr"
 
@@ -96,9 +97,12 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) {
 	if err := c.Watch(source.Kind[client.Object](operatorCache, &corev1.ConfigMap{}, handler.EnqueueRequestForOwner(scheme, mapper, &operatorv1.DNS{}))); err != nil {
 		return nil, err
 	}
+	if err := c.Watch(source.Kind[client.Object](operatorCache, &networkingv1.NetworkPolicy{}, handler.EnqueueRequestForOwner(scheme, mapper, &operatorv1.DNS{}))); err != nil {
+		return nil, err
+	}
 
 	objectToDNS := func(context.Context, client.Object) []reconcile.Request {
-		return []reconcile.Request{{DefaultDNSNamespaceName()}}
+		return []reconcile.Request{{NamespacedName: DefaultDNSNamespaceName()}}
 	}
 	isInNS := func(namespace string) func(o client.Object) bool {
 		return func(o client.Object) bool {
@@ -417,6 +421,18 @@ func (r *reconciler) ensureDNSNamespace() error {
 		logrus.Infof("created serviceaccount %s", nodeResolverServiceAccountName)
 	}
 
+	// Ensure the deny all network policy is present for the dns namespace
+	np := manifests.NetworkPolicyDenyAll()
+	if err := r.client.Get(context.TODO(), types.NamespacedName{Namespace: np.Namespace, Name: np.Name}, np); err != nil {
+		if !errors.IsNotFound(err) {
+			return fmt.Errorf("failed to get network policy deny all: %v", err)
+		}
+		if err := r.client.Create(context.TODO(), np); err != nil {
+			return fmt.Errorf("failed to create dns deny all network policy %s/%s: %v", np.Namespace, np.Name, err)
+		}
+		logrus.Infof("created dns deny all network policy %s/%s", np.Namespace, np.Name)
+	}
+
 	return nil
 }
 
@@ -534,6 +550,9 @@ func (r *reconciler) ensureDNS(dns *operatorv1.DNS, reconcileResult *reconcile.R
 		if _, _, err := r.ensureDNSConfigMap(dns, clusterDomain, cmMap); err != nil {
 			errs = append(errs, fmt.Errorf("failed to create configmap for dns %s: %v", dns.Name, err))
 		}
+		if _, _, err := r.ensureDNSNetworkPolicy(dns); err != nil {
+			errs = append(errs, fmt.Errorf("failed to ensure networkpolicy for dns %s: %v", dns.Name, err))
+		}
 		if haveSvc, svc, err := r.ensureDNSService(dns, clusterIP, daemonsetRef); err != nil {
 			// Set clusterIP to an empty string to cause ClusterOperator to report
 			// Available=False and Degraded=True.
diff --git a/pkg/operator/controller/controller_dns_networkpolicy.go b/pkg/operator/controller/controller_dns_networkpolicy.go
@@ -0,0 +1,121 @@
+package controller
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	operatorv1 "github.com/openshift/api/operator/v1"
+	"github.com/openshift/cluster-dns-operator/pkg/manifests"
+
+	"github.com/sirupsen/logrus"
+
+	networkingv1 "k8s.io/api/networking/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// ensureDNSNetworkPolicy ensures that a NetworkPolicy exists for the given DNS.
+func (r *reconciler) ensureDNSNetworkPolicy(dns *operatorv1.DNS) (bool, *networkingv1.NetworkPolicy, error) {
+	haveNP, current, err := r.currentDNSNetworkPolicy(dns)
+	if err != nil {
+		return false, nil, err
+	}
+
+	desired := desiredDNSNetworkPolicy(dns)
+
+	switch {
+	case !haveNP:
+		if err := r.client.Create(context.TODO(), desired); err != nil {
+			return false, nil, fmt.Errorf("failed to create dns networkpolicy: %v", err)
+		}
+		logrus.Infof("created dns networkpolicy: %s/%s", desired.Namespace, desired.Name)
+		return r.currentDNSNetworkPolicy(dns)
+	case haveNP:
+		if updated, err := r.updateDNSNetworkPolicy(current, desired); err != nil {
+			return true, current, err
+		} else if updated {
+			return r.currentDNSNetworkPolicy(dns)
+		}
+	}
+	return true, current, nil
+}
+
+func (r *reconciler) currentDNSNetworkPolicy(dns *operatorv1.DNS) (bool, *networkingv1.NetworkPolicy, error) {
+	current := &networkingv1.NetworkPolicy{}
+	if err := r.client.Get(context.TODO(), DNSNetworkPolicyName(dns), current); err != nil {
+		if errors.IsNotFound(err) {
+			return false, nil, nil
+		}
+		return false, nil, err
+	}
+	return true, current, nil
+}
+
+func desiredDNSNetworkPolicy(dns *operatorv1.DNS) *networkingv1.NetworkPolicy {
+	np := manifests.DNSNetworkPolicy()
+
+	name := DNSNetworkPolicyName(dns)
+	np.Namespace = name.Namespace
+	np.Name = name.Name
+	np.SetOwnerReferences([]metav1.OwnerReference{dnsOwnerRef(dns)})
+
+	if np.Labels == nil {
+		np.Labels = map[string]string{}
+	}
+	np.Labels[manifests.OwningDNSLabel] = DNSDaemonSetLabel(dns)
+
+	// Ensure pod selector matches the DNS daemonset pods for this instance.
+	if sel := DNSDaemonSetPodSelector(dns); sel != nil {
+		np.Spec.PodSelector = *sel
+	}
+
+	return np
+}
+
+func (r *reconciler) updateDNSNetworkPolicy(current, desired *networkingv1.NetworkPolicy) (bool, error) {
+	changed, updated := networkPolicyChanged(current, desired)
+	if !changed {
+		return false, nil
+	}
+
+	// Diff before updating because the client may mutate the object.
+	diff := cmp.Diff(current, updated, cmpopts.EquateEmpty())
+	if err := r.client.Update(context.TODO(), updated); err != nil {
+		return false, fmt.Errorf("failed to update dns networkpolicy %s/%s: %v", updated.Namespace, updated.Name, err)
+	}
+	logrus.Infof("updated dns networkpolicy %s/%s: %v", updated.Namespace, updated.Name, diff)
+	return true, nil
+}
+
+func networkPolicyChanged(current, expected *networkingv1.NetworkPolicy) (bool, *networkingv1.NetworkPolicy) {
+	// Ignore fields that the API, other controllers, or users may modify.
+	npCmpOpts := []cmp.Option{
+		cmpopts.EquateEmpty(),
+	}
+
+	currentLabels := current.Labels
+	if currentLabels == nil {
+		currentLabels = map[string]string{}
+	}
+	expectedLabels := expected.Labels
+	if expectedLabels == nil {
+		expectedLabels = map[string]string{}
+	}
+
+	if cmp.Equal(current.Spec, expected.Spec, npCmpOpts...) && cmp.Equal(currentLabels, expectedLabels, npCmpOpts...) {
+		return false, nil
+	}
+
+	updated := current.DeepCopy()
+	updated.Spec = expected.Spec
+	if updated.Labels == nil {
+		updated.Labels = map[string]string{}
+	}
+	for k, v := range expectedLabels {
+		updated.Labels[k] = v
+	}
+
+	return true, updated
+}
diff --git a/pkg/operator/controller/controller_dns_networkpolicy_test.go b/pkg/operator/controller/controller_dns_networkpolicy_test.go
diff --git a/pkg/operator/controller/names.go b/pkg/operator/controller/names.go
