NE-1476: Added network policies for the router

knobunc · knobunc · commit b8fc7f7ce7c1 · 2025-08-11T11:36:12.000-04:00
Added the framework for network policies for the router. The operator has a deny all network policy that for the openshift-ingress-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-ingress namespace. Then for each ingresscontroller that it manages it installs an allow policy for ingress for http and https traffic and metrics. It has to allow ingress from the router pods to any IP because the route endpoints can be at any ip or pod. 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
@@ -35,9 +35,9 @@ if [[ "${TEMP_COMMIT}" == "true" ]]; then
 fi
 
 cp -R manifests/* $MANIFESTS
-cat manifests/02-deployment.yaml | sed "s~openshift/origin-cluster-ingress-operator:latest~$REPO:$TAG~" > "$MANIFESTS/02-deployment.yaml"
+cat manifests/03-deployment.yaml | sed "s~openshift/origin-cluster-ingress-operator:latest~$REPO:$TAG~" > "$MANIFESTS/03-deployment.yaml"
 # To simulate CVO, ClusterOperator resource need to be created by the operator.
-rm $MANIFESTS/03-cluster-operator.yaml
+rm $MANIFESTS/04-cluster-operator.yaml
 
 echo "Pushed $REPO:$TAG"
 echo "Install manifests using:"
diff --git a/manifests/00-cluster-role.yaml b/manifests/00-cluster-role.yaml
@@ -155,6 +155,7 @@ rules:
   - networking.k8s.io
   resources:
   - ingressclasses
+  - networkpolicies
   verbs:
   - '*'
 
diff --git a/manifests/01-role.yaml b/manifests/01-role.yaml
@@ -37,6 +37,13 @@ rules:
   - services
   verbs:
   - "*"
+
+- apiGroups:
+  - networking.k8s.io
+  resources:
+  - networkpolicies
+  verbs:
+  - "*"
 ---
 # Role for the operator to delete Role and RoleBindings
 # in the openshift-config namespace.
diff --git a/manifests/02-network-policy.yaml b/manifests/02-network-policy.yaml
@@ -0,0 +1,43 @@
+# We control the namespace, deny anything we do not explicitly allow
+apiVersion: networking.k8s.io/v1
+kind: NetworkPolicy
+metadata:
+  name: ingress-operator-deny-all
+  namespace: openshift-ingress-operator
+  annotations:
+    capability.openshift.io/name: Ingress
+    include.release.openshift.io/self-managed-high-availability: "true"
+    include.release.openshift.io/single-node-developer: "true"
+spec:
+  podSelector: {}
+  policyTypes:
+  - Ingress
+  - Egress
+---
+### Allow the operator needs to talk to the apiserver and dns,
+### but it also needs to be able to configure external DNS and the endpoints
+### are only known at run time.  So it needs wildcard egress.
+### Allow access to the metrics ports on the operators
+apiVersion: networking.k8s.io/v1
+kind: NetworkPolicy
+metadata:
+  name: ingress-operator-allow
+  namespace: openshift-ingress-operator
+  annotations:
+    include.release.openshift.io/self-managed-high-availability: "true"
+    include.release.openshift.io/single-node-developer: "true"
+spec:
+  podSelector:
+    matchLabels:
+      name: ingress-operator
+  policyTypes:
+  - Egress
+  - Ingress
+  egress:
+  - to:
+    - ipBlock:
+        cidr: 0.0.0.0/0
+  ingress:
+  - ports:
+    - protocol: TCP
+      port: 9393
diff --git a/manifests/03-deployment-ibm-cloud-managed.yaml b/manifests/03-deployment-ibm-cloud-managed.yaml
diff --git a/manifests/03-deployment.yaml b/manifests/03-deployment.yaml
diff --git a/manifests/04-cluster-operator.yaml b/manifests/04-cluster-operator.yaml
diff --git a/pkg/manifests/assets/router/networkpolicy-allow.yaml b/pkg/manifests/assets/router/networkpolicy-allow.yaml
@@ -0,0 +1,25 @@
+# Router Pods
+#  Egress to api server and everything (routes set endpoints, so can be literally anything)
+#  Ingress to http on port 80 and https on port 443 (TCP) and to metrics (1936)
+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: 80
+    - protocol: TCP
+      port: 443
+    - protocol: TCP
+      port: 1936
+  egress:
+  - to:
+    - ipBlock:
+        cidr: 0.0.0.0/0
+  policyTypes:
+  - Ingress
+  - Egress
diff --git a/pkg/manifests/assets/router/networkpolicy-deny-all.yaml b/pkg/manifests/assets/router/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-ingress-deny-all
+    namespace: openshift-ingress
+spec:
+ podSelector: {}
+ policyTypes:
+ - Ingress
+ - Egress
diff --git a/pkg/manifests/manifests.go b/pkg/manifests/manifests.go
@@ -13,6 +13,7 @@ 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"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 
@@ -24,13 +25,15 @@ import (
 )
 
 const (
-	RouterNamespaceAsset          = "assets/router/namespace.yaml"
-	RouterServiceAccountAsset     = "assets/router/service-account.yaml"
-	RouterClusterRoleAsset        = "assets/router/cluster-role.yaml"
-	RouterClusterRoleBindingAsset = "assets/router/cluster-role-binding.yaml"
-	RouterDeploymentAsset         = "assets/router/deployment.yaml"
-	RouterServiceInternalAsset    = "assets/router/service-internal.yaml"
-	RouterServiceCloudAsset       = "assets/router/service-cloud.yaml"
+	RouterNamespaceAsset            = "assets/router/namespace.yaml"
+	RouterServiceAccountAsset       = "assets/router/service-account.yaml"
+	RouterClusterRoleAsset          = "assets/router/cluster-role.yaml"
+	RouterClusterRoleBindingAsset   = "assets/router/cluster-role-binding.yaml"
+	RouterDeploymentAsset           = "assets/router/deployment.yaml"
+	RouterServiceInternalAsset      = "assets/router/service-internal.yaml"
+	RouterServiceCloudAsset         = "assets/router/service-cloud.yaml"
+	RouterNetworkPolicyDenyAllAsset = "assets/router/networkpolicy-deny-all.yaml"
+	RouterNetworkPolicyAllowAsset   = "assets/router/networkpolicy-allow.yaml"
 
 	MetricsClusterRoleAsset        = "assets/router/metrics/cluster-role.yaml"
 	MetricsClusterRoleBindingAsset = "assets/router/metrics/cluster-role-binding.yaml"
@@ -314,6 +317,22 @@ func GatewayAPIViewClusterRole() *rbacv1.ClusterRole {
 	return clusterRole
 }
 
+func RouterNetworkPolicyDenyAll() *networkingv1.NetworkPolicy {
+	networkPolicy, err := NewNetworkPolicy(MustAssetReader(RouterNetworkPolicyDenyAllAsset))
+	if err != nil {
+		panic(err)
+	}
+	return networkPolicy
+}
+
+func RouterNetworkPolicyAllow() *networkingv1.NetworkPolicy {
+	networkPolicy, err := NewNetworkPolicy(MustAssetReader(RouterNetworkPolicyAllowAsset))
+	if err != nil {
+		panic(err)
+	}
+	return networkPolicy
+}
+
 func NewServiceAccount(manifest io.Reader) (*corev1.ServiceAccount, error) {
 	sa := corev1.ServiceAccount{}
 	if err := yaml.NewYAMLOrJSONDecoder(manifest, 100).Decode(&sa); err != nil {
@@ -404,6 +423,15 @@ func NewRoute(manifest io.Reader) (*routev1.Route, error) {
 	return &o, nil
 }
 
+func NewNetworkPolicy(manifest io.Reader) (*networkingv1.NetworkPolicy, error) {
+	o := networkingv1.NetworkPolicy{}
+	if err := yaml.NewYAMLOrJSONDecoder(manifest, 100).Decode(&o); err != nil {
+		return nil, err
+	}
+
+	return &o, nil
+}
+
 func NewCustomResourceDefinition(manifest io.Reader) (*apiextensionsv1.CustomResourceDefinition, error) {
 	o := apiextensionsv1.CustomResourceDefinition{}
 	if err := yaml.NewYAMLOrJSONDecoder(manifest, 100).Decode(&o); err != nil {
diff --git a/pkg/operator/controller/ingress/controller.go b/pkg/operator/controller/ingress/controller.go
@@ -24,6 +24,7 @@ import (
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	networkingv1 "k8s.io/api/networking/v1"
 
 	"k8s.io/client-go/tools/record"
 
@@ -114,6 +115,9 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) {
 	if err := c.Watch(source.Kind[client.Object](operatorCache, &corev1.Service{}, enqueueRequestForOwningIngressController(config.Namespace))); err != nil {
 		return nil, err
 	}
+	if err := c.Watch(source.Kind[client.Object](operatorCache, &networkingv1.NetworkPolicy{}, enqueueRequestForOwningIngressController(config.Namespace))); err != nil {
+		return nil, err
+	}
 	// Add watch for deleted pods specifically for ensuring ingress deletion.
 	if err := c.Watch(source.Kind[client.Object](operatorCache, &corev1.Pod{}, enqueueRequestForOwningIngressController(config.Namespace), predicate.Funcs{
 		CreateFunc:  func(e event.CreateEvent) bool { return false },
@@ -1078,6 +1082,10 @@ func (r *reconciler) ensureIngressController(ci *operatorv1.IngressController, d
 		return fmt.Errorf("failed to ensure namespace: %v", err)
 	}
 
+	if _, _, err := r.ensureRouterNamespaceNetworkPolicy(ci); err != nil {
+		return fmt.Errorf("failed to ensure default namespacenetwork policy: %v", err)
+	}
+
 	if err := r.ensureRouterServiceAccount(); err != nil {
 		return fmt.Errorf("failed to ensure service account: %v", err)
 	}
@@ -1086,6 +1094,10 @@ func (r *reconciler) ensureIngressController(ci *operatorv1.IngressController, d
 		return fmt.Errorf("failed to ensure cluster role binding: %v", err)
 	}
 
+	if err := r.ensureRouterNetworkPolicy(ci); err != nil {
+		return fmt.Errorf("failed to ensure network policy: %v", err)
+	}
+
 	var errs []error
 	if _, _, err := r.ensureServiceCAConfigMap(); err != nil {
 		// Even if we were unable to create the configmap at this time,
diff --git a/pkg/operator/controller/ingress/namespace.go b/pkg/operator/controller/ingress/namespace.go
@@ -8,10 +8,12 @@ import (
 	"github.com/google/go-cmp/cmp/cmpopts"
 
 	corev1 "k8s.io/api/core/v1"
+	networkingv1 "k8s.io/api/networking/v1"
 
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 
+	operatorv1 "github.com/openshift/api/operator/v1"
 	"github.com/openshift/cluster-ingress-operator/pkg/manifests"
 	operatorcontroller "github.com/openshift/cluster-ingress-operator/pkg/operator/controller"
 )
@@ -155,3 +157,17 @@ func (r *reconciler) ensureRouterClusterRoleBinding() error {
 	}
 	return nil
 }
+
+func (r *reconciler) ensureRouterNamespaceNetworkPolicy(ci *operatorv1.IngressController) (bool, *networkingv1.NetworkPolicy, error) {
+	np := manifests.RouterNetworkPolicyDenyAll()
+	if err := r.client.Get(context.TODO(), types.NamespacedName{Namespace: np.Namespace, Name: np.Name}, np); err != nil {
+		if !errors.IsNotFound(err) {
+			return false, nil, fmt.Errorf("failed to get router network policy %s/%s: %v", np.Namespace, np.Name, err)
+		}
+		if err := r.client.Create(context.TODO(), np); err != nil {
+			return false, nil, fmt.Errorf("failed to create router network policy %s/%s: %v", np.Namespace, np.Name, err)
+		}
+		log.Info("created router network policy", "namespace", np.Namespace, "name", np.Name)
+	}
+	return true, np, nil
+}
diff --git a/pkg/operator/controller/ingress/networkpolicy.go b/pkg/operator/controller/ingress/networkpolicy.go
@@ -0,0 +1,121 @@
+package ingress
+
+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-ingress-operator/pkg/manifests"
+	controller "github.com/openshift/cluster-ingress-operator/pkg/operator/controller"
+
+	networkingv1 "k8s.io/api/networking/v1"
+
+	"k8s.io/apimachinery/pkg/api/errors"
+)
+
+// ensureRouterNetworkPolicy ensures the per-ingresscontroller NetworkPolicy that
+// allows ingress to router pods and egress to the network exists and is up to date.
+func (r *reconciler) ensureRouterNetworkPolicy(ic *operatorv1.IngressController) error {
+	desired := desiredRouterNetworkPolicy(ic)
+
+	have, current, err := r.currentRouterNetworkPolicy(ic)
+	if err != nil {
+		return err
+	}
+
+	switch {
+	case !have:
+		if err := r.client.Create(context.TODO(), desired); err != nil {
+			return fmt.Errorf("failed to create router network policy: %v", err)
+		}
+		log.Info("created router network policy", "networkpolicy", desired)
+		return nil
+	default:
+		if updated, err := r.updateRouterNetworkPolicy(current, desired); err != nil {
+			return fmt.Errorf("failed to update router network policy: %v", err)
+		} else if updated {
+			return nil
+		}
+	}
+
+	return nil
+}
+
+// desiredRouterNetworkPolicy returns the desired per-ingresscontroller NetworkPolicy.
+func desiredRouterNetworkPolicy(ic *operatorv1.IngressController) *networkingv1.NetworkPolicy {
+	np := manifests.RouterNetworkPolicyAllow()
+
+	name := controller.RouterNetworkPolicyName(ic)
+	np.Namespace = name.Namespace
+	np.Name = name.Name
+
+	if np.Labels == nil {
+		np.Labels = map[string]string{}
+	}
+	np.Labels[manifests.OwningIngressControllerLabel] = ic.Name
+
+	// Select router pods for this ingresscontroller.
+	np.Spec.PodSelector = *controller.IngressControllerDeploymentPodSelector(ic)
+
+	return np
+}
+
+// currentRouterNetworkPolicy returns the current per-ingresscontroller NetworkPolicy, if it exists.
+func (r *reconciler) currentRouterNetworkPolicy(ic *operatorv1.IngressController) (bool, *networkingv1.NetworkPolicy, error) {
+	current := &networkingv1.NetworkPolicy{}
+	if err := r.client.Get(context.TODO(), controller.RouterNetworkPolicyName(ic), current); err != nil {
+		if errors.IsNotFound(err) {
+			return false, nil, nil
+		}
+		return false, nil, err
+	}
+	return true, current, nil
+}
+
+// updateRouterNetworkPolicy updates the NetworkPolicy if it differs from the desired state.
+func (r *reconciler) updateRouterNetworkPolicy(current, desired *networkingv1.NetworkPolicy) (bool, error) {
+	changed, updated := routerNetworkPolicyChanged(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, err
+	}
+	log.Info("updated router network policy", "namespace", updated.Namespace, "name", updated.Name, "diff", diff)
+	return true, nil
+}
+
+// routerNetworkPolicyChanged checks whether the current NetworkPolicy matches the expected
+// state and, if not, returns an updated NetworkPolicy.
+func routerNetworkPolicyChanged(current, expected *networkingv1.NetworkPolicy) (bool, *networkingv1.NetworkPolicy) {
+	changed := false
+
+	if !cmp.Equal(current.Spec, expected.Spec, cmpopts.EquateEmpty()) {
+		changed = true
+	}
+
+	// Ensure the owning-ingresscontroller label value is as expected while preserving other labels.
+	if current.Labels == nil || current.Labels[manifests.OwningIngressControllerLabel] != expected.Labels[manifests.OwningIngressControllerLabel] {
+		changed = true
+	}
+
+	if !changed {
+		return false, nil
+	}
+
+	updated := current.DeepCopy()
+	updated.Spec = expected.Spec
+
+	if updated.Labels == nil {
+		updated.Labels = map[string]string{}
+	}
+	updated.Labels[manifests.OwningIngressControllerLabel] = expected.Labels[manifests.OwningIngressControllerLabel]
+
+	return true, updated
+}
diff --git a/pkg/operator/controller/ingress/networkpolicy_test.go b/pkg/operator/controller/ingress/networkpolicy_test.go
diff --git a/pkg/operator/controller/names.go b/pkg/operator/controller/names.go
