Skip to content

Commit 095baff

Browse files
committed
NE-1476 - Added network policies for DNS
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
1 parent 48ebc12 commit 095baff

11 files changed

+325
-1
lines changed
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
### Allow the operators to talk to the apiserver and dns
2+
### Allow access to the metrics ports on the operators
3+
apiVersion: networking.k8s.io/v1
4+
kind: NetworkPolicy
5+
metadata:
6+
name: dns-operator-allow
7+
namespace: openshift-dns-operator
8+
annotations:
9+
include.release.openshift.io/self-managed-high-availability: "true"
10+
include.release.openshift.io/single-node-developer: "true"
11+
spec:
12+
podSelector:
13+
matchLabels:
14+
name: dns-operator
15+
policyTypes:
16+
- Egress
17+
- Ingress
18+
egress:
19+
- ports:
20+
- protocol: TCP
21+
port: 6443
22+
- protocol: TCP
23+
port: 53
24+
- protocol: UDP
25+
port: 53
26+
ingress:
27+
- ports:
28+
- protocol: TCP
29+
port: 9393
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
# We control the namespace, deny anything we do not explicitly allow
2+
apiVersion: networking.k8s.io/v1
3+
kind: NetworkPolicy
4+
metadata:
5+
name: dns-operator-deny-all
6+
namespace: openshift-dns-operator
7+
annotations:
8+
include.release.openshift.io/self-managed-high-availability: "true"
9+
include.release.openshift.io/single-node-developer: "true"
10+
spec:
11+
podSelector: {}
12+
policyTypes:
13+
- Ingress
14+
- Egress

manifests/0000_70_dns-operator_01-role.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,10 @@ rules:
2727
- services
2828
verbs:
2929
- "*"
30+
31+
- apiGroups:
32+
- networking.k8s.io
33+
resources:
34+
- networkpolicies
35+
verbs:
36+
- "*"
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# DNS Pods
2+
# Egress to api server and upstream dns (can be wildcarded, so allow any egress)
3+
# Ingress to dns on port 5353 (TCP and UDP) and to metrics (9154)
4+
apiVersion: networking.k8s.io/v1
5+
kind: NetworkPolicy
6+
# name, namespace,labels and annotations are set at runtime
7+
spec:
8+
podSelector:
9+
# matchLabels are set at runtime
10+
matchLabels: {}
11+
ingress:
12+
- ports:
13+
- protocol: TCP
14+
port: 9154
15+
- protocol: UDP
16+
port: 5353
17+
- protocol: TCP
18+
port: 5353
19+
policyTypes:
20+
- Ingress
21+
- Egress
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# Default deny all policy for all pods in the namespace
2+
apiVersion: networking.k8s.io/v1
3+
kind: NetworkPolicy
4+
metadata:
5+
name: openshift-dns-deny-all
6+
namespace: openshift-dns
7+
spec:
8+
podSelector: {}
9+
policyTypes:
10+
- Ingress
11+
- Egress

pkg/manifests/manifests.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,22 @@ import (
77

88
appsv1 "k8s.io/api/apps/v1"
99
corev1 "k8s.io/api/core/v1"
10+
networkingv1 "k8s.io/api/networking/v1"
1011
rbacv1 "k8s.io/api/rbac/v1"
1112

1213
"k8s.io/apimachinery/pkg/util/yaml"
1314
)
1415

1516
const (
17+
NetworkPolicyDenyAllAsset = "assets/networkpolicy-deny-all.yaml"
18+
1619
DNSNamespaceAsset = "assets/dns/namespace.yaml"
1720
DNSServiceAccountAsset = "assets/dns/service-account.yaml"
1821
DNSClusterRoleAsset = "assets/dns/cluster-role.yaml"
1922
DNSClusterRoleBindingAsset = "assets/dns/cluster-role-binding.yaml"
2023
DNSDaemonSetAsset = "assets/dns/daemonset.yaml"
2124
DNSServiceAsset = "assets/dns/service.yaml"
25+
DNSNetworkPolicyAsset = "assets/dns/networkpolicy-allow.yaml"
2226

2327
MetricsClusterRoleAsset = "assets/dns/metrics/cluster-role.yaml"
2428
MetricsClusterRoleBindingAsset = "assets/dns/metrics/cluster-role-binding.yaml"
@@ -55,6 +59,14 @@ func MustAssetReader(asset string) io.Reader {
5559
return bytes.NewReader(MustAsset(asset))
5660
}
5761

62+
func NetworkPolicyDenyAll() *networkingv1.NetworkPolicy {
63+
np, err := NewNetworkPolicy(MustAssetReader(NetworkPolicyDenyAllAsset))
64+
if err != nil {
65+
panic(err)
66+
}
67+
return np
68+
}
69+
5870
func DNSNamespace() *corev1.Namespace {
5971
ns, err := NewNamespace(MustAssetReader(DNSNamespaceAsset))
6072
if err != nil {
@@ -103,6 +115,14 @@ func DNSService() *corev1.Service {
103115
return s
104116
}
105117

118+
func DNSNetworkPolicy() *networkingv1.NetworkPolicy {
119+
np, err := NewNetworkPolicy(MustAssetReader(DNSNetworkPolicyAsset))
120+
if err != nil {
121+
panic(err)
122+
}
123+
return np
124+
}
125+
106126
func MetricsClusterRole() *rbacv1.ClusterRole {
107127
cr, err := NewClusterRole(MustAssetReader(MetricsClusterRoleAsset))
108128
if err != nil {
@@ -220,3 +240,11 @@ func NewNamespace(manifest io.Reader) (*corev1.Namespace, error) {
220240
}
221241
return &ns, nil
222242
}
243+
244+
func NewNetworkPolicy(manifest io.Reader) (*networkingv1.NetworkPolicy, error) {
245+
np := networkingv1.NetworkPolicy{}
246+
if err := yaml.NewYAMLOrJSONDecoder(manifest, 100).Decode(&np); err != nil {
247+
return nil, err
248+
}
249+
return &np, nil
250+
}

pkg/manifests/manifests_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,15 @@ import (
55
)
66

77
func TestManifests(t *testing.T) {
8+
NetworkPolicyDenyAll()
9+
810
DNSServiceAccount()
911
DNSClusterRole()
1012
DNSClusterRoleBinding()
1113
DNSNamespace()
1214
DNSDaemonSet()
1315
DNSService()
16+
DNSNetworkPolicy()
1417

1518
MetricsClusterRole()
1619
MetricsClusterRoleBinding()

pkg/operator/controller/controller.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818

1919
appsv1 "k8s.io/api/apps/v1"
2020
corev1 "k8s.io/api/core/v1"
21+
networkingv1 "k8s.io/api/networking/v1"
2122

2223
"github.com/apparentlymart/go-cidr/cidr"
2324

@@ -96,9 +97,12 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) {
9697
if err := c.Watch(source.Kind[client.Object](operatorCache, &corev1.ConfigMap{}, handler.EnqueueRequestForOwner(scheme, mapper, &operatorv1.DNS{}))); err != nil {
9798
return nil, err
9899
}
100+
if err := c.Watch(source.Kind[client.Object](operatorCache, &networkingv1.NetworkPolicy{}, handler.EnqueueRequestForOwner(scheme, mapper, &operatorv1.DNS{}))); err != nil {
101+
return nil, err
102+
}
99103

100104
objectToDNS := func(context.Context, client.Object) []reconcile.Request {
101-
return []reconcile.Request{{DefaultDNSNamespaceName()}}
105+
return []reconcile.Request{{NamespacedName: DefaultDNSNamespaceName()}}
102106
}
103107
isInNS := func(namespace string) func(o client.Object) bool {
104108
return func(o client.Object) bool {
@@ -417,6 +421,18 @@ func (r *reconciler) ensureDNSNamespace() error {
417421
logrus.Infof("created serviceaccount %s", nodeResolverServiceAccountName)
418422
}
419423

424+
// Ensure the deny all network policy is present for the dns namespace
425+
np := manifests.NetworkPolicyDenyAll()
426+
if err := r.client.Get(context.TODO(), types.NamespacedName{Namespace: np.Namespace, Name: np.Name}, np); err != nil {
427+
if !errors.IsNotFound(err) {
428+
return fmt.Errorf("failed to get network policy deny all: %v", err)
429+
}
430+
if err := r.client.Create(context.TODO(), np); err != nil {
431+
return fmt.Errorf("failed to create dns deny all network policy %s/%s: %v", np.Namespace, np.Name, err)
432+
}
433+
logrus.Infof("created dns deny all network policy %s/%s", np.Namespace, np.Name)
434+
}
435+
420436
return nil
421437
}
422438

@@ -534,6 +550,9 @@ func (r *reconciler) ensureDNS(dns *operatorv1.DNS, reconcileResult *reconcile.R
534550
if _, _, err := r.ensureDNSConfigMap(dns, clusterDomain, cmMap); err != nil {
535551
errs = append(errs, fmt.Errorf("failed to create configmap for dns %s: %v", dns.Name, err))
536552
}
553+
if _, _, err := r.ensureDNSNetworkPolicy(dns); err != nil {
554+
errs = append(errs, fmt.Errorf("failed to ensure networkpolicy for dns %s: %v", dns.Name, err))
555+
}
537556
if haveSvc, svc, err := r.ensureDNSService(dns, clusterIP, daemonsetRef); err != nil {
538557
// Set clusterIP to an empty string to cause ClusterOperator to report
539558
// Available=False and Degraded=True.
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
package controller
2+
3+
import (
4+
"context"
5+
"fmt"
6+
7+
"github.com/google/go-cmp/cmp"
8+
"github.com/google/go-cmp/cmp/cmpopts"
9+
operatorv1 "github.com/openshift/api/operator/v1"
10+
"github.com/openshift/cluster-dns-operator/pkg/manifests"
11+
12+
"github.com/sirupsen/logrus"
13+
14+
networkingv1 "k8s.io/api/networking/v1"
15+
"k8s.io/apimachinery/pkg/api/errors"
16+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
17+
)
18+
19+
// ensureDNSNetworkPolicy ensures that a NetworkPolicy exists for the given DNS.
20+
func (r *reconciler) ensureDNSNetworkPolicy(dns *operatorv1.DNS) (bool, *networkingv1.NetworkPolicy, error) {
21+
haveNP, current, err := r.currentDNSNetworkPolicy(dns)
22+
if err != nil {
23+
return false, nil, err
24+
}
25+
26+
desired := desiredDNSNetworkPolicy(dns)
27+
28+
switch {
29+
case !haveNP:
30+
if err := r.client.Create(context.TODO(), desired); err != nil {
31+
return false, nil, fmt.Errorf("failed to create dns networkpolicy: %v", err)
32+
}
33+
logrus.Infof("created dns networkpolicy: %s/%s", desired.Namespace, desired.Name)
34+
return r.currentDNSNetworkPolicy(dns)
35+
case haveNP:
36+
if updated, err := r.updateDNSNetworkPolicy(current, desired); err != nil {
37+
return true, current, err
38+
} else if updated {
39+
return r.currentDNSNetworkPolicy(dns)
40+
}
41+
}
42+
return true, current, nil
43+
}
44+
45+
func (r *reconciler) currentDNSNetworkPolicy(dns *operatorv1.DNS) (bool, *networkingv1.NetworkPolicy, error) {
46+
current := &networkingv1.NetworkPolicy{}
47+
if err := r.client.Get(context.TODO(), DNSNetworkPolicyName(dns), current); err != nil {
48+
if errors.IsNotFound(err) {
49+
return false, nil, nil
50+
}
51+
return false, nil, err
52+
}
53+
return true, current, nil
54+
}
55+
56+
func desiredDNSNetworkPolicy(dns *operatorv1.DNS) *networkingv1.NetworkPolicy {
57+
np := manifests.DNSNetworkPolicy()
58+
59+
name := DNSNetworkPolicyName(dns)
60+
np.Namespace = name.Namespace
61+
np.Name = name.Name
62+
np.SetOwnerReferences([]metav1.OwnerReference{dnsOwnerRef(dns)})
63+
64+
if np.Labels == nil {
65+
np.Labels = map[string]string{}
66+
}
67+
np.Labels[manifests.OwningDNSLabel] = DNSDaemonSetLabel(dns)
68+
69+
// Ensure pod selector matches the DNS daemonset pods for this instance.
70+
if sel := DNSDaemonSetPodSelector(dns); sel != nil {
71+
np.Spec.PodSelector = *sel
72+
}
73+
74+
return np
75+
}
76+
77+
func (r *reconciler) updateDNSNetworkPolicy(current, desired *networkingv1.NetworkPolicy) (bool, error) {
78+
changed, updated := networkPolicyChanged(current, desired)
79+
if !changed {
80+
return false, nil
81+
}
82+
83+
// Diff before updating because the client may mutate the object.
84+
diff := cmp.Diff(current, updated, cmpopts.EquateEmpty())
85+
if err := r.client.Update(context.TODO(), updated); err != nil {
86+
return false, fmt.Errorf("failed to update dns networkpolicy %s/%s: %v", updated.Namespace, updated.Name, err)
87+
}
88+
logrus.Infof("updated dns networkpolicy %s/%s: %v", updated.Namespace, updated.Name, diff)
89+
return true, nil
90+
}
91+
92+
func networkPolicyChanged(current, expected *networkingv1.NetworkPolicy) (bool, *networkingv1.NetworkPolicy) {
93+
// Ignore fields that the API, other controllers, or users may modify.
94+
npCmpOpts := []cmp.Option{
95+
cmpopts.EquateEmpty(),
96+
}
97+
98+
currentLabels := current.Labels
99+
if currentLabels == nil {
100+
currentLabels = map[string]string{}
101+
}
102+
expectedLabels := expected.Labels
103+
if expectedLabels == nil {
104+
expectedLabels = map[string]string{}
105+
}
106+
107+
if cmp.Equal(current.Spec, expected.Spec, npCmpOpts...) && cmp.Equal(currentLabels, expectedLabels, npCmpOpts...) {
108+
return false, nil
109+
}
110+
111+
updated := current.DeepCopy()
112+
updated.Spec = expected.Spec
113+
if updated.Labels == nil {
114+
updated.Labels = map[string]string{}
115+
}
116+
for k, v := range expectedLabels {
117+
updated.Labels[k] = v
118+
}
119+
120+
return true, updated
121+
}

0 commit comments

Comments
 (0)