NE-1476: Added network policies for DNS

manifests/0000_70_dns-operator_00-cluster-role.yaml

@@ -139,3 +139,15 @@ rules:
   - get
   - update
   - patch
+
+- apiGroups:
+  - networking.k8s.io
+  resources:
+  - networkpolicies
+  verbs:
+  - get
+  - list
+  - create
+  - update
+  - watch
+  - delete

manifests/0000_70_dns-operator_01-network-policy.yaml

@@ -0,0 +1,55 @@
+# 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
+  - to:
+    - namespaceSelector:
+        matchLabels:
+          kubernetes.io/metadata.name: openshift-apiserver
+    - podSelector:
+        matchLabels:
+          apiserver: "true"
+  ingress:
+  - from:
+    - namespaceSelector:
+        matchLabels:
+          kubernetes.io/metadata.name: openshift-monitoring
+    ports:
+    - protocol: TCP
+      port: 9393

manifests/0000_70_dns-operator_01-role.yaml

@@ -27,3 +27,15 @@ rules:
   - services
   verbs:
   - "*"
+
+- apiGroups:
+  - networking.k8s.io
+  resources:
+  - networkpolicies
+  verbs:
+  - get
+  - list
+  - create
+  - update
+  - watch
+  - delete

pkg/manifests/assets/dns/networkpolicy-allow.yaml

@@ -0,0 +1,43 @@
+# 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:
+  # Everyone needs to be able to access the dns ports
+  - ports:
+    - protocol: UDP
+      port: 5353
+    - protocol: TCP
+      port: 5353
+  # Allow monitoring to hit the metrics port
+  - ports:
+    - protocol: TCP
+      port: 9154
+    from:
+    - namespaceSelector:
+        matchLabels:
+          kubernetes.io/metadata.name: openshift-monitoring
+  # Allow the dns operator namespaces to hit the healthcheck ports
+  - ports:
+    - protocol: TCP
+      port: 8080
+    - protocol: TCP
+      port: 8181
+    from:
+    - namespaceSelector:
+        matchLabels:
+          kubernetes.io/metadata.name: openshift-dns-operator
+          kubernetes.io/metadata.name: openshift-dns
+  egress:
+  - to:
+    - ipBlock:
+        cidr: 0.0.0.0/0
+  policyTypes:
+  - Ingress
+  - Egress

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

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
+}

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()

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.

pkg/operator/controller/controller_dns_networkpolicy.go

@@ -0,0 +1,119 @@
+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
+	updated.Labels = map[string]string{}
+	for k, v := range expectedLabels {
+		updated.Labels[k] = v
+	}
+
+	return true, updated
+}