From ceb627d68b70f0fadad790193048c4d30b2140b5 Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Thu, 20 Aug 2026 11:08:07 +0100 Subject: [PATCH 1/2] ADD: network policy for operand Network policies created and managed for authorino and limitador. The ports are linked to default ports or the ports defined in the respected CRs. Tests have also being added. Signed-off-by: Jim Fitzpatrick --- api/v1beta1/topology.go | 47 + ...adrant-operator.clusterserviceversion.yaml | 109 +-- .../templates/manifests.yaml | 12 + cmd/main.go | 2 + config/rbac/role.yaml | 12 + .../controller/networkpolicy_reconciler.go | 432 ++++++++++ .../networkpolicy_reconciler_test.go | 816 ++++++++++++++++++ internal/controller/state_of_the_world.go | 20 + 8 files changed, 1357 insertions(+), 93 deletions(-) create mode 100644 internal/controller/networkpolicy_reconciler.go create mode 100644 internal/controller/networkpolicy_reconciler_test.go diff --git a/api/v1beta1/topology.go b/api/v1beta1/topology.go index ac5ae1603..c893c7a83 100644 --- a/api/v1beta1/topology.go +++ b/api/v1beta1/topology.go @@ -7,6 +7,7 @@ import ( "github.com/kuadrant/policy-machinery/machinery" "github.com/samber/lo" appsv1 "k8s.io/api/apps/v1" + networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -25,6 +26,9 @@ var ( DeploymentGroupKind = appsv1.SchemeGroupVersion.WithKind("Deployment").GroupKind() DeploymentsResource = appsv1.SchemeGroupVersion.WithResource("deployments") + + NetworkPolicyGroupKind = schema.GroupKind{Group: networkingv1.SchemeGroupVersion.Group, Kind: "NetworkPolicy"} + NetworkPolicyResource = networkingv1.SchemeGroupVersion.WithResource("networkpolicies") ) func LinkKuadrantToGatewayClasses(objs controller.Store) machinery.LinkFunc { @@ -71,6 +75,21 @@ func LinkKuadrantToAuthorino(objs controller.Store) machinery.LinkFunc { } } +func LinkAuthorinoToDeployment(objs controller.Store) machinery.LinkFunc { + authorinos := utils.Map(objs.FilterByGroupKind(AuthorinoGroupKind), ControllerObjectToMachineryObject) + + return machinery.LinkFunc{ + From: AuthorinoGroupKind, + To: DeploymentGroupKind, + Func: func(deployment machinery.Object) []machinery.Object { + return lo.Filter(authorinos, func(authorino machinery.Object, _ int) bool { + // the name of the deployment is hardcoded. This deployment is owned by the authorino operator. + return authorino.GetNamespace() == deployment.GetNamespace() && deployment.GetName() == "authorino" + }) + }, + } +} + func LinkLimitadorToDeployment(objs controller.Store) machinery.LinkFunc { limitadors := utils.Map(objs.FilterByGroupKind(LimitadorGroupKind), ControllerObjectToMachineryObject) @@ -126,6 +145,34 @@ func LinkKuadrantToPodMonitor(objs controller.Store) machinery.LinkFunc { } } +func LinkAuthorinoToNetworkPolicy(objs controller.Store) machinery.LinkFunc { + authorinos := utils.Map(objs.FilterByGroupKind(AuthorinoGroupKind), ControllerObjectToMachineryObject) + + return machinery.LinkFunc{ + From: AuthorinoGroupKind, + To: NetworkPolicyGroupKind, + Func: func(networkPolicy machinery.Object) []machinery.Object { + return lo.Filter(authorinos, func(authorino machinery.Object, _ int) bool { + return authorino.GetNamespace() == networkPolicy.GetNamespace() && networkPolicy.GetName() == "kuadrant-authorino" + }) + }, + } +} + +func LinkLimitadorToNetworkPolicy(objs controller.Store) machinery.LinkFunc { + limitadors := utils.Map(objs.FilterByGroupKind(LimitadorGroupKind), ControllerObjectToMachineryObject) + + return machinery.LinkFunc{ + From: LimitadorGroupKind, + To: NetworkPolicyGroupKind, + Func: func(networkPolicy machinery.Object) []machinery.Object { + return lo.Filter(limitadors, func(limitador machinery.Object, _ int) bool { + return limitador.GetNamespace() == networkPolicy.GetNamespace() && networkPolicy.GetName() == "kuadrant-limitador" + }) + }, + } +} + func ControllerObjectToMachineryObject(cObj controller.Object) machinery.Object { if mObj, ok := cObj.(machinery.Object); ok { return mObj diff --git a/bundle/manifests/kuadrant-operator.clusterserviceversion.yaml b/bundle/manifests/kuadrant-operator.clusterserviceversion.yaml index b838aee26..5cadd4adc 100644 --- a/bundle/manifests/kuadrant-operator.clusterserviceversion.yaml +++ b/bundle/manifests/kuadrant-operator.clusterserviceversion.yaml @@ -225,7 +225,7 @@ metadata: categories: Integration & Delivery console.openshift.io/plugins: '["kuadrant-console-plugin"]' containerImage: quay.io/kuadrant/kuadrant-operator:latest - createdAt: "2026-08-27T13:56:33Z" + createdAt: "2026-08-25T15:38:41Z" description: A Kubernetes Operator to manage the lifecycle of the Kuadrant system operators.operatorframework.io/builder: operator-sdk-v1.33.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v4 @@ -261,9 +261,6 @@ spec: kind: DNSPolicy name: dnspolicies.kuadrant.io version: v1 - - kind: KuadrantControlPlane - name: kuadrantcontrolplanes.kuadrant.io - version: v1alpha1 - description: Kuadrant configures installations of Kuadrant Service Protection components displayName: Kuadrant @@ -415,39 +412,17 @@ spec: - apiGroups: - "" resources: - - namespaces - verbs: - - get - - list - - watch - - apiGroups: - - "" - - events.k8s.io - resources: - events verbs: - create - patch - apiGroups: - - apiextensions.k8s.io - resources: - - customresourcedefinitions - verbs: - - create - - list - - watch - - apiGroups: - - apiextensions.k8s.io - resourceNames: - - dnshealthcheckprobes.kuadrant.io - - dnsrecords.kuadrant.io + - "" resources: - - customresourcedefinitions + - namespaces verbs: - get - list - - patch - - update - watch - apiGroups: - apps @@ -461,12 +436,6 @@ spec: - patch - update - watch - - apiGroups: - - authentication.k8s.io - resources: - - tokenreviews - verbs: - - create - apiGroups: - authorino.kuadrant.io resources: @@ -479,12 +448,6 @@ spec: - patch - update - watch - - apiGroups: - - authorization.k8s.io - resources: - - subjectaccessreviews - verbs: - - create - apiGroups: - cert-manager.io resources: @@ -642,7 +605,6 @@ spec: - kuadrant.io resources: - authpolicies - - kuadrantcontrolplanes verbs: - create - get @@ -666,7 +628,6 @@ spec: resources: - authpolicies/status - dnspolicies/status - - kuadrantcontrolplanes/status - kuadrants/status - ratelimitpolicies/status - tlspolicies/status @@ -745,9 +706,9 @@ spec: - update - watch - apiGroups: - - operator.authorino.kuadrant.io + - networking.k8s.io resources: - - authorinos + - networkpolicies verbs: - create - delete @@ -757,42 +718,9 @@ spec: - update - watch - apiGroups: - - rbac.authorization.k8s.io - resourceNames: - - dns-operator-manager-rolebinding - - dns-operator-remote-cluster-rolebinding - resources: - - clusterrolebindings - verbs: - - delete - - get - - patch - - update - - apiGroups: - - rbac.authorization.k8s.io - resources: - - clusterrolebindings - - clusterroles - verbs: - - create - - apiGroups: - - rbac.authorization.k8s.io - resourceNames: - - dns-operator-manager-role - - dns-operator-remote-cluster-role - resources: - - clusterroles - verbs: - - bind - - escalate - - get - - patch - - update - - apiGroups: - - rbac.authorization.k8s.io + - operator.authorino.kuadrant.io resources: - - rolebindings - - roles + - authorinos verbs: - create - delete @@ -854,8 +782,6 @@ spec: value: quay.io/kuadrant/console-plugin:v0.6.0 - name: RELATED_IMAGE_CONSOLE_PLUGIN_PF5 value: quay.io/kuadrant/console-plugin:v0.1.5-2 - - name: RELATED_IMAGE_DNS_OPERATOR - value: quay.io/kuadrant/dns-operator:latest - name: OPERATOR_NAMESPACE valueFrom: fieldRef: @@ -938,6 +864,14 @@ spec: - patch serviceAccountName: developer-portal-controller-manager - rules: + - apiGroups: + - "" + resources: + - secrets + verbs: + - get + - list + - watch - apiGroups: - "" resources: @@ -969,15 +903,6 @@ spec: verbs: - create - patch - - apiGroups: - - operators.coreos.com - resources: - - subscriptions - - clusterserviceversions - verbs: - - get - - list - - delete serviceAccountName: kuadrant-operator-controller-manager strategy: deployment installModes: @@ -1012,7 +937,7 @@ spec: - email: didier@redhat.com name: Didier Di Cesare maturity: alpha - minKubeVersion: 1.25.0 + minKubeVersion: 1.19.0 provider: name: Red Hat url: https://github.com/Kuadrant/kuadrant-operator @@ -1027,6 +952,4 @@ spec: name: console-plugin-sdk1 - image: quay.io/kuadrant/console-plugin:v0.1.5-2 name: console-plugin-pf5 - - image: quay.io/kuadrant/dns-operator:latest - name: dns-operator version: 0.0.0 diff --git a/charts/kuadrant-operator/templates/manifests.yaml b/charts/kuadrant-operator/templates/manifests.yaml index 73d566fd6..34cc9bb99 100644 --- a/charts/kuadrant-operator/templates/manifests.yaml +++ b/charts/kuadrant-operator/templates/manifests.yaml @@ -14816,6 +14816,18 @@ rules: - patch - update - watch +- apiGroups: + - networking.k8s.io + resources: + - networkpolicies + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - operator.authorino.kuadrant.io resources: diff --git a/cmd/main.go b/cmd/main.go index e4eaae84e..d7a75cb14 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -43,6 +43,7 @@ import ( istiosecurity "istio.io/client-go/pkg/apis/security/v1" corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" k8sruntime "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -100,6 +101,7 @@ func init() { utilruntime.Must(consolev1.AddToScheme(scheme)) utilruntime.Must(monitoringv1.AddToScheme(scheme)) utilruntime.Must(istiosecurity.AddToScheme(scheme)) + utilruntime.Must(networkingv1.AddToScheme(scheme)) //+kubebuilder:scaffold:scheme sync = zapcore.Lock(zapcore.AddSync(os.Stdout)) diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index a16145f82..a7d1e9ff9 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -351,6 +351,18 @@ rules: - patch - update - watch +- apiGroups: + - networking.k8s.io + resources: + - networkpolicies + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - operator.authorino.kuadrant.io resources: diff --git a/internal/controller/networkpolicy_reconciler.go b/internal/controller/networkpolicy_reconciler.go new file mode 100644 index 000000000..10e0d9edb --- /dev/null +++ b/internal/controller/networkpolicy_reconciler.go @@ -0,0 +1,432 @@ +package controllers + +import ( + "context" + "errors" + "fmt" + "reflect" + "slices" + "sync" + + "github.com/go-logr/logr" + authorinooperatorv1beta1 "github.com/kuadrant/authorino-operator/api/v1beta1" + limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" + "github.com/kuadrant/policy-machinery/controller" + "github.com/kuadrant/policy-machinery/machinery" + "github.com/samber/lo" + "go.opentelemetry.io/otel/codes" + "go.opentelemetry.io/otel/trace" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/dynamic" + "k8s.io/utils/ptr" + + "github.com/kuadrant/kuadrant-operator/api/v1beta1" +) + +const ( + NetworkPolicyReconcilerName = "NetworkPolicyReconciler" + AuthorinoNetworkPolicy = "kuadrant-authorino" + LimitadorNetworkPolicy = "kuadrant-limitador" +) + +// writeChecks used to decide if a resource should be write to a cluster via create or update +type writeChecks = struct { + Create bool + Update bool +} + +type NetworkPolicyReconciler struct { + Client *dynamic.DynamicClient +} + +//+kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,verbs=get;list;watch;create;update;patch;delete + +func NewNetworkPolicyReconciler(client *dynamic.DynamicClient) *NetworkPolicyReconciler { + return &NetworkPolicyReconciler{Client: client} +} + +func (r *NetworkPolicyReconciler) Subscription() *controller.Subscription { + return &controller.Subscription{ + ReconcileFunc: r.Reconcile, + Events: []controller.ResourceEventMatcher{ + {Kind: &v1beta1.KuadrantGroupKind, EventType: ptr.To(controller.CreateEvent)}, + {Kind: &v1beta1.KuadrantGroupKind, EventType: ptr.To(controller.DeleteEvent)}, + {Kind: &v1beta1.AuthorinoGroupKind, EventType: ptr.To(controller.CreateEvent)}, + {Kind: &v1beta1.AuthorinoGroupKind, EventType: ptr.To(controller.UpdateEvent)}, + {Kind: &v1beta1.LimitadorGroupKind, EventType: ptr.To(controller.CreateEvent)}, + {Kind: &v1beta1.LimitadorGroupKind, EventType: ptr.To(controller.UpdateEvent)}, + {Kind: &v1beta1.NetworkPolicyGroupKind}, + {Kind: &machinery.GatewayGroupKind, EventType: ptr.To(controller.CreateEvent)}, + {Kind: &machinery.GatewayGroupKind, EventType: ptr.To(controller.DeleteEvent)}, + }, + } +} + +func (r *NetworkPolicyReconciler) Reconcile(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, state *sync.Map) error { + span := trace.SpanFromContext(ctx) + defer span.End() + + logger := controller.LoggerFromContext(ctx).WithName(NetworkPolicyReconcilerName) + logger.Info("reconciling networkPolicy resource", "status", "started") + defer logger.Info("reconciling networkPolicy resource", "status", "completed") + + policies := getNetworkPolicies(topology) + + kObj := GetKuadrantFromTopology(topology, state) + if kObj == nil { + span.AddEvent("no kuadrant object found") + span.SetStatus(codes.Ok, "no kuadrant resource") + return nil + } + + var errs []error + // ------------------------------------------------------------------------------------------------------------- + span.AddEvent("setting authorino network policy") + update := false + + authorinoObj := GetAuthorinoFromTopology(topology, state) + + minAuthorinoNetworkPolicy := generateAuthorinoNetworkPolicy(kObj, authorinoObj, topology) + + var existingAuthorinoNetworkPolicy *networkingv1.NetworkPolicy + for _, policy := range policies { + if policy.GetName() == AuthorinoNetworkPolicy { + existingAuthorinoNetworkPolicy = policy + break + } + } + + desiredAuthorinoNetworkPolicy, update := mergeNetworkPolicy(*minAuthorinoNetworkPolicy, existingAuthorinoNetworkPolicy) + + if authorinoObj != nil { + ownerRef := metav1.OwnerReference{ + APIVersion: authorinoObj.GroupVersionKind().GroupVersion().String(), + Kind: authorinoObj.Kind, + Name: authorinoObj.GetName(), + UID: authorinoObj.GetUID(), + BlockOwnerDeletion: new(true), + Controller: new(true), + } + + var existingOwnerRefs []metav1.OwnerReference + + if existingAuthorinoNetworkPolicy != nil { + existingOwnerRefs = existingAuthorinoNetworkPolicy.GetOwnerReferences() + } + if !slices.ContainsFunc(existingOwnerRefs, func(ref metav1.OwnerReference) bool { + return ref.UID == ownerRef.UID + }) { + existingOwnerRefs = append(existingOwnerRefs, ownerRef) + } + + desiredAuthorinoNetworkPolicy.SetOwnerReferences(existingOwnerRefs) + } + + err := r.writePolicyToCluster(ctx, logger, span, desiredAuthorinoNetworkPolicy, writeChecks{ + Create: existingAuthorinoNetworkPolicy == nil, + Update: update, + }) + if err != nil { + logger.Error(err, "failed to write authorino network policy to cluster") + errs = append(errs, err) + } + + // ------------------------------------------------------------------------------------------------------------- + + span.AddEvent("setting limitador network policy") + + lObj := GetLimitadorFromTopology(topology, state) + minLimitadorNetworkPolicy := generateLimitadorNetworkPolicy(kObj, lObj, topology) + + var existingLimitadorNetworkPolicy *networkingv1.NetworkPolicy + for _, policy := range policies { + if policy.GetName() == LimitadorNetworkPolicy { + existingLimitadorNetworkPolicy = policy + break + } + } + + desiredLimitadorNetworkPolicy, update := mergeNetworkPolicy(*minLimitadorNetworkPolicy, existingLimitadorNetworkPolicy) + + if lObj != nil { + ownerRef := metav1.OwnerReference{ + APIVersion: lObj.GroupVersionKind().GroupVersion().String(), + Kind: lObj.Kind, + Name: lObj.GetName(), + UID: lObj.GetUID(), + BlockOwnerDeletion: new(true), + Controller: new(true), + } + + var existingOwnerRefs []metav1.OwnerReference + + if existingLimitadorNetworkPolicy != nil { + existingOwnerRefs = existingLimitadorNetworkPolicy.GetOwnerReferences() + } + if !slices.ContainsFunc(existingOwnerRefs, func(ref metav1.OwnerReference) bool { + return ref.UID == ownerRef.UID + }) { + existingOwnerRefs = append(existingOwnerRefs, ownerRef) + } + + desiredLimitadorNetworkPolicy.SetOwnerReferences(existingOwnerRefs) + } + + err = r.writePolicyToCluster(ctx, logger, span, desiredLimitadorNetworkPolicy, writeChecks{ + Create: existingLimitadorNetworkPolicy == nil, + Update: update, + }) + if err != nil { + logger.Error(err, "failed to write limitador network policy to cluster") + errs = append(errs, err) + } + // ------------------------------------------------------------------------------------------------------------- + + if len(errs) > 0 { + span.SetStatus(codes.Error, "reconciliation completed with errors") + } else { + span.SetStatus(codes.Ok, "") + } + return errors.Join(errs...) +} + +func (r *NetworkPolicyReconciler) writePolicyToCluster(ctx context.Context, logger logr.Logger, span trace.Span, networkPolicy *networkingv1.NetworkPolicy, check writeChecks) error { + if networkPolicy == nil { + return fmt.Errorf("networkPolicy is nil") + } + + desiredNetworkPolicyUnstructured, err := controller.Destruct(networkPolicy) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, "failed to destruct NetworkPolicy") + logger.Error(err, "failed to destruct NetworkPolicy object", "networkPolicy", networkPolicy) + return err + } + if check.Create { + logger.Info("creating network policy") + if _, err = r.Client.Resource(v1beta1.NetworkPolicyResource).Namespace(networkPolicy.GetNamespace()).Create(ctx, desiredNetworkPolicyUnstructured, metav1.CreateOptions{}); err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, "failed to create NetworkPolicy") + logger.Error(err, "failed to create NetworkPolicy object", "networkPolicy", desiredNetworkPolicyUnstructured.Object) + return err + } + } else if check.Update { + if _, err = r.Client.Resource(v1beta1.NetworkPolicyResource).Namespace(networkPolicy.GetNamespace()).Update(ctx, desiredNetworkPolicyUnstructured, metav1.UpdateOptions{}); err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, "failed to update NetworkPolicy") + logger.Error(err, "failed to update NetworkPolicy object", "networkPolicy", desiredNetworkPolicyUnstructured.Object) + return err + } + } + return nil +} + +func getNetworkPolicies(topology *machinery.Topology) []*networkingv1.NetworkPolicy { + policies := topology.Objects().Items(func(object machinery.Object) bool { + return object.GroupVersionKind().GroupKind().Kind == v1beta1.NetworkPolicyGroupKind.Kind + }) + + if policies == nil { + return nil + } + + output := make([]*networkingv1.NetworkPolicy, len(policies)) + + for idx, policy := range policies { + p, ok := policy.(*controller.RuntimeObject).Object.(*networkingv1.NetworkPolicy) + if ok && p != nil { + output[idx] = p + } + } + + return output +} + +func gatewayNamespacePeers(topology *machinery.Topology) []networkingv1.NetworkPolicyPeer { + gateways := topology.Targetables().Items(func(o machinery.Object) bool { + return o.GroupVersionKind().Kind == machinery.GatewayGroupKind.Kind + }) + var seen []string + var peers []networkingv1.NetworkPolicyPeer + for _, gateway := range gateways { + ns := gateway.GetNamespace() + if !slices.Contains(seen, ns) { + seen = append(seen, ns) + peers = append(peers, networkingv1.NetworkPolicyPeer{ + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"kubernetes.io/metadata.name": ns}, + }, + }) + } + } + return peers +} + +func generateAuthorinoNetworkPolicy(kObj *v1beta1.Kuadrant, aObj *authorinooperatorv1beta1.Authorino, topology *machinery.Topology) *networkingv1.NetworkPolicy { + fromNamespaces := gatewayNamespacePeers(topology) + + labels := linkedDeploymentLabels(&controller.RuntimeObject{Object: aObj}, topology) + + if labels == nil { + labels = map[string]string{"kuadrant.io/managed": "true"} + } + + // These default port values are hardcode into the authServerCmd in the authorino repo + // https://github.com/Kuadrant/authorino/blob/58fecc6cdec38376fa7dba5638f1f7ecb6964cd0/main.go#L178-L218 + gRPCport := 50051 + HTTPport := 5051 + OIDCdiscoveryPort := 8083 + + if aObj != nil { + if aObj.Spec.Listener.Ports.GRPC != nil { + gRPCport = int(*aObj.Spec.Listener.Ports.GRPC) + } + if aObj.Spec.Listener.Ports.HTTP != nil { + HTTPport = int(*aObj.Spec.Listener.Ports.HTTP) + } + if aObj.Spec.OIDCServer.Port != nil { + OIDCdiscoveryPort = int(*aObj.Spec.OIDCServer.Port) + } + } + + return &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: AuthorinoNetworkPolicy, + Namespace: kObj.GetNamespace(), + Labels: CommonLabels(), + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: labels, + }, + PolicyTypes: []networkingv1.PolicyType{"Ingress"}, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + // gRPC ext-auth from Envoy + ingressRule(fromNamespaces, gRPCport), + // HTTP ext-auth from gateway + ingressRule(fromNamespaces, HTTPport), + // OIDC discovery endpoint + ingressRule([]networkingv1.NetworkPolicyPeer{}, OIDCdiscoveryPort), + }, + }, + } +} + +func ingressRule(from []networkingv1.NetworkPolicyPeer, port int) networkingv1.NetworkPolicyIngressRule { + return networkingv1.NetworkPolicyIngressRule{ + From: from, + Ports: networkPolicyPortTCP(port), + } +} + +func networkPolicyPortTCP(port int) []networkingv1.NetworkPolicyPort { + return []networkingv1.NetworkPolicyPort{ + { + Protocol: ptr.To(corev1.ProtocolTCP), + Port: new(intstr.FromInt(port))}, + } +} + +func mergeNetworkPolicy(desired networkingv1.NetworkPolicy, current *networkingv1.NetworkPolicy) (*networkingv1.NetworkPolicy, bool) { + changed := false + + if current == nil { + return &desired, true + } + // check desiredLabels + desiredLabels := desired.GetLabels() + currentLabels := current.GetLabels() + for key, dValue := range desiredLabels { + if currentLabels[key] != dValue { + current.Labels[key] = dValue + changed = true + } + } + + if !reflect.DeepEqual(desired.Spec.Ingress, current.Spec.Ingress) { + current.Spec.Ingress = desired.Spec.Ingress + changed = true + } + + if !reflect.DeepEqual(desired.Spec.PodSelector, current.Spec.PodSelector) { + current.Spec.PodSelector = desired.Spec.PodSelector + changed = true + } + + return current, changed +} + +func generateLimitadorNetworkPolicy(kObj *v1beta1.Kuadrant, lObj *limitadorv1alpha1.Limitador, topology *machinery.Topology) *networkingv1.NetworkPolicy { + fromNamespaces := gatewayNamespacePeers(topology) + + labels := linkedDeploymentLabels(&controller.RuntimeObject{Object: lObj}, topology) + + if labels == nil { + labels = map[string]string{"kuadrant.io/managed": "true"} + } + + // These default port values are hardcode into the impl Configuration in the limitador repo + // https://github.com/Kuadrant/limitador/blob/f73e5f4b3d9af3756d4d772b35d2798693b961f9/limitador-server/src/config.rs#L101-L103 + gRPCport := 8081 + HTTPport := 8080 + + if lObj != nil { + gRPCport = int(lObj.GRPCPort()) + HTTPport = int(lObj.HTTPPort()) + } + + return &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: LimitadorNetworkPolicy, + Namespace: kObj.GetNamespace(), + Labels: CommonLabels(), + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: labels, + }, + PolicyTypes: []networkingv1.PolicyType{"Ingress"}, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + // gRPC rate limit checks + ingressRule(fromNamespaces, gRPCport), + // HTTP rate limit checks + ingressRule(fromNamespaces, HTTPport), + }, + }, + } +} + +func linkedDeploymentLabels(resource *controller.RuntimeObject, topology *machinery.Topology) map[string]string { + if resource == nil { + return nil + } + + // Check for typed nil - when Object interface contains a nil pointer + v := reflect.ValueOf(resource.Object) + if !v.IsValid() || (v.Kind() == reflect.Pointer && v.IsNil()) { + return nil + } + + deployments := lo.FilterMap(topology.All().Children(resource), func(child machinery.Object, _ int) (*appsv1.Deployment, bool) { + if child.GroupVersionKind().GroupKind() != v1beta1.DeploymentGroupKind { + return nil, false + } + runtimeObj, ok := child.(*controller.RuntimeObject) + if !ok { + return nil, false + } + deployment, ok := runtimeObj.Object.(*appsv1.Deployment) + return deployment, ok + }) + + if len(deployments) == 1 { + deployment := deployments[0] + return deployment.Spec.Template.GetLabels() + } + + return nil +} diff --git a/internal/controller/networkpolicy_reconciler_test.go b/internal/controller/networkpolicy_reconciler_test.go new file mode 100644 index 000000000..2d7dbf658 --- /dev/null +++ b/internal/controller/networkpolicy_reconciler_test.go @@ -0,0 +1,816 @@ +//go:build unit + +package controllers + +import ( + "maps" + "testing" + + authorinooperatorv1beta1 "github.com/kuadrant/authorino-operator/api/v1beta1" + limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" + "github.com/kuadrant/policy-machinery/controller" + "github.com/kuadrant/policy-machinery/machinery" + "gotest.tools/assert" + is "gotest.tools/assert/cmp" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" + gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" + + kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1" +) + +func TestNetworkPolicyPortTCP(t *testing.T) { + port := 8080 + result := networkPolicyPortTCP(port) + + assert.Assert(t, is.Len(result, 1), "should return single element slice") + assert.DeepEqual(t, result[0].Protocol, ptr.To(corev1.ProtocolTCP)) + assert.DeepEqual(t, result[0].Port, new(intstr.FromInt(port))) +} + +func TestIngressRule(t *testing.T) { + t.Run("empty peers", func(t *testing.T) { + port := 8080 + result := ingressRule([]networkingv1.NetworkPolicyPeer{}, port) + + assert.Assert(t, is.Len(result.From, 0), "From should be empty") + assert.Assert(t, is.Len(result.Ports, 1), "Ports should have one entry") + assert.DeepEqual(t, result.Ports[0].Port, new(intstr.FromInt(port))) + }) + + t.Run("single peer", func(t *testing.T) { + port := 9090 + peers := []networkingv1.NetworkPolicyPeer{ + { + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"kubernetes.io/metadata.name": "ns1"}, + }, + }, + } + result := ingressRule(peers, port) + + assert.Assert(t, is.Len(result.From, 1), "From should have one peer") + assert.Assert(t, is.Len(result.Ports, 1), "Ports should have one entry") + assert.DeepEqual(t, result.From[0], peers[0]) + assert.DeepEqual(t, result.Ports[0].Port, new(intstr.FromInt(port))) + }) + + t.Run("multiple peers", func(t *testing.T) { + port := 7070 + peers := []networkingv1.NetworkPolicyPeer{ + { + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"kubernetes.io/metadata.name": "ns1"}, + }, + }, + { + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"kubernetes.io/metadata.name": "ns2"}, + }, + }, + } + result := ingressRule(peers, port) + + assert.Assert(t, is.Len(result.From, 2), "From should have two peers") + assert.Assert(t, is.Len(result.Ports, 1), "Ports should have one entry") + assert.DeepEqual(t, result.From[0], peers[0]) + assert.DeepEqual(t, result.From[1], peers[1]) + assert.DeepEqual(t, result.Ports[0].Port, new(intstr.FromInt(port))) + }) +} + +func TestGatewayNamespacePeers(t *testing.T) { + t.Run("empty topology", func(t *testing.T) { + topology, err := machinery.NewTopology() + assert.NilError(t, err) + + result := gatewayNamespacePeers(topology) + assert.Assert(t, is.Len(result, 0), "should return empty slice for empty topology") + }) + + t.Run("single gateway", func(t *testing.T) { + gateway := &gatewayapiv1.Gateway{ + TypeMeta: metav1.TypeMeta{ + Kind: "Gateway", + APIVersion: "gateway.networking.k8s.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "gw1", + Namespace: "ns1", + }, + } + topology, err := machinery.NewTopology( + machinery.WithTargetables(&machinery.Gateway{Gateway: gateway}), + ) + assert.NilError(t, err) + + result := gatewayNamespacePeers(topology) + assert.Assert(t, is.Len(result, 1), "should return one peer for single gateway") + assert.DeepEqual(t, result[0].NamespaceSelector.MatchLabels, map[string]string{"kubernetes.io/metadata.name": "ns1"}) + }) + + t.Run("two gateways same namespace (dedup)", func(t *testing.T) { + gateway1 := &gatewayapiv1.Gateway{ + TypeMeta: metav1.TypeMeta{ + Kind: "Gateway", + APIVersion: "gateway.networking.k8s.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "gw1", + Namespace: "ns1", + }, + } + gateway2 := &gatewayapiv1.Gateway{ + TypeMeta: metav1.TypeMeta{ + Kind: "Gateway", + APIVersion: "gateway.networking.k8s.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "gw2", + Namespace: "ns1", + }, + } + topology, err := machinery.NewTopology( + machinery.WithTargetables( + &machinery.Gateway{Gateway: gateway1}, + &machinery.Gateway{Gateway: gateway2}, + ), + ) + assert.NilError(t, err) + + result := gatewayNamespacePeers(topology) + assert.Assert(t, is.Len(result, 1), "should deduplicate same namespace") + assert.DeepEqual(t, result[0].NamespaceSelector.MatchLabels, map[string]string{"kubernetes.io/metadata.name": "ns1"}) + }) + + t.Run("two gateways different namespaces", func(t *testing.T) { + gateway1 := &gatewayapiv1.Gateway{ + TypeMeta: metav1.TypeMeta{ + Kind: "Gateway", + APIVersion: "gateway.networking.k8s.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "gw1", + Namespace: "ns1", + }, + } + gateway2 := &gatewayapiv1.Gateway{ + TypeMeta: metav1.TypeMeta{ + Kind: "Gateway", + APIVersion: "gateway.networking.k8s.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "gw2", + Namespace: "ns2", + }, + } + topology, err := machinery.NewTopology( + machinery.WithTargetables( + &machinery.Gateway{Gateway: gateway1}, + &machinery.Gateway{Gateway: gateway2}, + ), + ) + assert.NilError(t, err) + + result := gatewayNamespacePeers(topology) + assert.Assert(t, is.Len(result, 2), "should return two peers for different namespaces") + // Check that both namespaces are present (order not guaranteed) + namespaces := make(map[string]bool) + for _, peer := range result { + ns := peer.NamespaceSelector.MatchLabels["kubernetes.io/metadata.name"] + namespaces[ns] = true + } + assert.Assert(t, namespaces["ns1"], "should contain ns1") + assert.Assert(t, namespaces["ns2"], "should contain ns2") + }) +} + +func TestMergeNetworkPolicy(t *testing.T) { + t.Run("nil current", func(t *testing.T) { + desired := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-policy", + Namespace: "test-ns", + Labels: map[string]string{"app": "test"}, + }, + } + result, changed := mergeNetworkPolicy(*desired, nil) + + assert.Assert(t, changed, "should report changed=true") + assert.DeepEqual(t, result, desired) + }) + + t.Run("identical", func(t *testing.T) { + policy := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-policy", + Namespace: "test-ns", + Labels: map[string]string{"app": "test"}, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "pod"}, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + {Ports: networkPolicyPortTCP(8080)}, + }, + }, + } + desired := policy.DeepCopy() + current := policy.DeepCopy() + + result, changed := mergeNetworkPolicy(*desired, current) + + assert.Assert(t, !changed, "should report changed=false for identical policies") + assert.DeepEqual(t, result, current) + }) + + t.Run("different labels", func(t *testing.T) { + desired := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-policy", + Namespace: "test-ns", + Labels: map[string]string{"app": "new-value"}, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{}, + Ingress: []networkingv1.NetworkPolicyIngressRule{}, + }, + } + current := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-policy", + Namespace: "test-ns", + Labels: map[string]string{"app": "old-value"}, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{}, + Ingress: []networkingv1.NetworkPolicyIngressRule{}, + }, + } + + result, changed := mergeNetworkPolicy(*desired, current) + + assert.Assert(t, changed, "should report changed=true for different labels") + assert.Equal(t, result.Labels["app"], "new-value") + }) + + t.Run("different ingress", func(t *testing.T) { + desired := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-policy", + Namespace: "test-ns", + Labels: map[string]string{"app": "test"}, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{}, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + {Ports: networkPolicyPortTCP(9090)}, + }, + }, + } + current := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-policy", + Namespace: "test-ns", + Labels: map[string]string{"app": "test"}, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{}, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + {Ports: networkPolicyPortTCP(8080)}, + }, + }, + } + + result, changed := mergeNetworkPolicy(*desired, current) + + assert.Assert(t, changed, "should report changed=true for different ingress") + assert.DeepEqual(t, result.Spec.Ingress, desired.Spec.Ingress) + }) + + t.Run("different pod selector", func(t *testing.T) { + desired := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-policy", + Namespace: "test-ns", + Labels: map[string]string{"app": "test"}, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "new-pod"}, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{}, + }, + } + current := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-policy", + Namespace: "test-ns", + Labels: map[string]string{"app": "test"}, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "old-pod"}, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{}, + }, + } + + result, changed := mergeNetworkPolicy(*desired, current) + + assert.Assert(t, changed, "should report changed=true for different pod selector") + assert.DeepEqual(t, result.Spec.PodSelector, desired.Spec.PodSelector) + }) + + t.Run("current has extra labels not in desired", func(t *testing.T) { + desired := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-policy", + Namespace: "test-ns", + Labels: map[string]string{"app": "test"}, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{}, + Ingress: []networkingv1.NetworkPolicyIngressRule{}, + }, + } + current := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-policy", + Namespace: "test-ns", + Labels: map[string]string{"app": "test", "extra": "label"}, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{}, + Ingress: []networkingv1.NetworkPolicyIngressRule{}, + }, + } + + result, changed := mergeNetworkPolicy(*desired, current) + + assert.Assert(t, !changed, "should report changed=false when extra labels exist") + assert.Equal(t, result.Labels["app"], "test") + assert.Equal(t, result.Labels["extra"], "label", "extra labels should be retained") + }) +} + +func TestGenerateAuthorinoNetworkPolicy(t *testing.T) { + kuadrant := &kuadrantv1beta1.Kuadrant{ + TypeMeta: metav1.TypeMeta{ + Kind: "Kuadrant", + APIVersion: "kuadrant.io/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "kuadrant", + Namespace: "kuadrant-system", + }, + } + + t.Run("nil authorino - default ports", func(t *testing.T) { + topology, err := machinery.NewTopology() + assert.NilError(t, err) + + result := generateAuthorinoNetworkPolicy(kuadrant, nil, topology) + + assert.Equal(t, result.Name, AuthorinoNetworkPolicy) + assert.Equal(t, result.Namespace, "kuadrant-system") + assert.DeepEqual(t, result.Spec.PodSelector.MatchLabels, map[string]string{"kuadrant.io/managed": "true"}) + assert.Assert(t, is.Len(result.Spec.Ingress, 3), "should have 3 ingress rules") + + // Verify gRPC port (default 50051) + assert.DeepEqual(t, result.Spec.Ingress[0].Ports[0].Port, new(intstr.FromInt(50051))) + // Verify HTTP port (default 5051) + assert.DeepEqual(t, result.Spec.Ingress[1].Ports[0].Port, new(intstr.FromInt(5051))) + // Verify OIDC port (default 8083) + assert.DeepEqual(t, result.Spec.Ingress[2].Ports[0].Port, new(intstr.FromInt(8083))) + }) + + t.Run("authorino with custom ports", func(t *testing.T) { + authorino := &authorinooperatorv1beta1.Authorino{ + TypeMeta: metav1.TypeMeta{ + Kind: "Authorino", + APIVersion: "operator.authorino.kuadrant.io/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "authorino", + Namespace: "kuadrant-system", + }, + Spec: authorinooperatorv1beta1.AuthorinoSpec{ + Listener: authorinooperatorv1beta1.Listener{ + Ports: authorinooperatorv1beta1.Ports{ + GRPC: new(int32(9000)), + HTTP: new(int32(9001)), + }, + }, + OIDCServer: authorinooperatorv1beta1.OIDCServer{ + Port: new(int32(9002)), + }, + }, + } + topology, err := machinery.NewTopology() + assert.NilError(t, err) + + result := generateAuthorinoNetworkPolicy(kuadrant, authorino, topology) + + // Verify custom gRPC port + assert.DeepEqual(t, result.Spec.Ingress[0].Ports[0].Port, new(intstr.FromInt(9000))) + // Verify custom HTTP port + assert.DeepEqual(t, result.Spec.Ingress[1].Ports[0].Port, new(intstr.FromInt(9001))) + // Verify custom OIDC port + assert.DeepEqual(t, result.Spec.Ingress[2].Ports[0].Port, new(intstr.FromInt(9002))) + }) + + t.Run("gateway in topology - peers in gRPC and HTTP but not OIDC", func(t *testing.T) { + gateway := &gatewayapiv1.Gateway{ + TypeMeta: metav1.TypeMeta{ + Kind: "Gateway", + APIVersion: "gateway.networking.k8s.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "gw1", + Namespace: "gateway-ns", + }, + } + topology, err := machinery.NewTopology( + machinery.WithTargetables(&machinery.Gateway{Gateway: gateway}), + ) + assert.NilError(t, err) + + result := generateAuthorinoNetworkPolicy(kuadrant, nil, topology) + + // gRPC rule should have gateway peer + assert.Assert(t, is.Len(result.Spec.Ingress[0].From, 1), "gRPC should have gateway peer") + assert.DeepEqual(t, result.Spec.Ingress[0].From[0].NamespaceSelector.MatchLabels, + map[string]string{"kubernetes.io/metadata.name": "gateway-ns"}) + + // HTTP rule should have gateway peer + assert.Assert(t, is.Len(result.Spec.Ingress[1].From, 1), "HTTP should have gateway peer") + assert.DeepEqual(t, result.Spec.Ingress[1].From[0].NamespaceSelector.MatchLabels, + map[string]string{"kubernetes.io/metadata.name": "gateway-ns"}) + + // OIDC rule should NOT have gateway peer (empty From) + assert.Assert(t, is.Len(result.Spec.Ingress[2].From, 0), "OIDC should not have gateway peer") + }) + + t.Run("common labels are set", func(t *testing.T) { + topology, err := machinery.NewTopology() + assert.NilError(t, err) + + result := generateAuthorinoNetworkPolicy(kuadrant, nil, topology) + + commonLabels := CommonLabels() + for key, value := range commonLabels { + assert.Equal(t, result.Labels[key], value, "common label %s should be set", key) + } + }) +} + +func TestGenerateLimitadorNetworkPolicy(t *testing.T) { + kuadrant := &kuadrantv1beta1.Kuadrant{ + TypeMeta: metav1.TypeMeta{ + Kind: "Kuadrant", + APIVersion: "kuadrant.io/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "kuadrant", + Namespace: "kuadrant-system", + }, + } + + t.Run("nil limitador - default ports", func(t *testing.T) { + topology, err := machinery.NewTopology() + assert.NilError(t, err) + + result := generateLimitadorNetworkPolicy(kuadrant, nil, topology) + + assert.Equal(t, result.Name, LimitadorNetworkPolicy) + assert.Equal(t, result.Namespace, "kuadrant-system") + assert.DeepEqual(t, result.Spec.PodSelector.MatchLabels, map[string]string{"kuadrant.io/managed": "true"}) + assert.Assert(t, is.Len(result.Spec.Ingress, 2), "should have 2 ingress rules") + + // Verify gRPC port (default 8081) + assert.DeepEqual(t, result.Spec.Ingress[0].Ports[0].Port, new(intstr.FromInt(8081))) + // Verify HTTP port (default 8080) + assert.DeepEqual(t, result.Spec.Ingress[1].Ports[0].Port, new(intstr.FromInt(8080))) + }) + + t.Run("limitador with custom ports", func(t *testing.T) { + limitador := &limitadorv1alpha1.Limitador{ + TypeMeta: metav1.TypeMeta{ + Kind: "Limitador", + APIVersion: "limitador.kuadrant.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "limitador", + Namespace: "kuadrant-system", + }, + Spec: limitadorv1alpha1.LimitadorSpec{ + Listener: &limitadorv1alpha1.Listener{ + GRPC: &limitadorv1alpha1.TransportProtocol{Port: new(int32(7001))}, + HTTP: &limitadorv1alpha1.TransportProtocol{Port: new(int32(7002))}, + }, + }, + } + topology, err := machinery.NewTopology() + assert.NilError(t, err) + + result := generateLimitadorNetworkPolicy(kuadrant, limitador, topology) + + // Verify custom gRPC port + assert.DeepEqual(t, result.Spec.Ingress[0].Ports[0].Port, new(intstr.FromInt(7001))) + // Verify custom HTTP port + assert.DeepEqual(t, result.Spec.Ingress[1].Ports[0].Port, new(intstr.FromInt(7002))) + }) + + t.Run("gateway peers appear in both ingress rules", func(t *testing.T) { + gateway := &gatewayapiv1.Gateway{ + TypeMeta: metav1.TypeMeta{ + Kind: "Gateway", + APIVersion: "gateway.networking.k8s.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "gw1", + Namespace: "gateway-ns", + }, + } + topology, err := machinery.NewTopology( + machinery.WithTargetables(&machinery.Gateway{Gateway: gateway}), + ) + assert.NilError(t, err) + + result := generateLimitadorNetworkPolicy(kuadrant, nil, topology) + + // gRPC rule should have gateway peer + assert.Assert(t, is.Len(result.Spec.Ingress[0].From, 1), "gRPC should have gateway peer") + assert.DeepEqual(t, result.Spec.Ingress[0].From[0].NamespaceSelector.MatchLabels, + map[string]string{"kubernetes.io/metadata.name": "gateway-ns"}) + + // HTTP rule should have gateway peer + assert.Assert(t, is.Len(result.Spec.Ingress[1].From, 1), "HTTP should have gateway peer") + assert.DeepEqual(t, result.Spec.Ingress[1].From[0].NamespaceSelector.MatchLabels, + map[string]string{"kubernetes.io/metadata.name": "gateway-ns"}) + }) + + t.Run("common labels are set", func(t *testing.T) { + topology, err := machinery.NewTopology() + assert.NilError(t, err) + + result := generateLimitadorNetworkPolicy(kuadrant, nil, topology) + + commonLabels := CommonLabels() + for key, value := range commonLabels { + assert.Equal(t, result.Labels[key], value, "common label %s should be set", key) + } + }) +} + +func TestLinkedDeploymentLabels(t *testing.T) { + t.Run("nil resource", func(t *testing.T) { + topology, err := machinery.NewTopology() + assert.NilError(t, err) + + result := linkedDeploymentLabels(nil, topology) + assert.Assert(t, result == nil, "should return nil for nil resource") + }) + + t.Run("resource with no deployment children", func(t *testing.T) { + authorino := &authorinooperatorv1beta1.Authorino{ + TypeMeta: metav1.TypeMeta{ + Kind: "Authorino", + APIVersion: "operator.authorino.kuadrant.io/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "authorino", + Namespace: "kuadrant-system", + UID: "authorino-uid", + }, + } + authorinoRuntimeObj := &controller.RuntimeObject{Object: authorino} + + topology, err := machinery.NewTopology( + machinery.WithObjects(&controller.RuntimeObject{Object: authorino}), + ) + assert.NilError(t, err) + + result := linkedDeploymentLabels(authorinoRuntimeObj, topology) + assert.Assert(t, result == nil, "should return nil when no deployment children") + }) + + t.Run("resource with one deployment child linked via topology", func(t *testing.T) { + authorino := &authorinooperatorv1beta1.Authorino{ + TypeMeta: metav1.TypeMeta{ + Kind: "Authorino", + APIVersion: "operator.authorino.kuadrant.io/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "authorino", + Namespace: "kuadrant-system", + UID: "authorino-uid", + }, + } + deployment := &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: "Deployment", + APIVersion: "apps/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "authorino", + Namespace: "kuadrant-system", + UID: "deployment-uid", + }, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": "authorino", + "version": "v1", + }, + }, + }, + }, + } + + authorinoRuntimeObj := &controller.RuntimeObject{Object: authorino} + deploymentRuntimeObj := &controller.RuntimeObject{Object: deployment} + + // Create store and link the objects + store := make(controller.Store) + store[string(authorinoRuntimeObj.GetUID())] = authorinoRuntimeObj + store[string(deploymentRuntimeObj.GetUID())] = deploymentRuntimeObj + + linkFunc := kuadrantv1beta1.LinkAuthorinoToDeployment(store) + + topology, err := machinery.NewTopology( + machinery.WithObjects(authorinoRuntimeObj, deploymentRuntimeObj), + machinery.WithLinks(linkFunc), + ) + assert.NilError(t, err) + + result := linkedDeploymentLabels(authorinoRuntimeObj, topology) + + assert.Assert(t, result != nil, "should return labels for linked deployment") + expectedLabels := map[string]string{ + "app": "authorino", + "version": "v1", + } + assert.Assert(t, maps.Equal(result, expectedLabels), "should return pod template labels") + }) +} + +func TestGetNetworkPolicies(t *testing.T) { + t.Run("empty topology", func(t *testing.T) { + topology, err := machinery.NewTopology() + assert.NilError(t, err) + + result := getNetworkPolicies(topology) + assert.Assert(t, is.Len(result, 0), "should return empty slice for empty topology") + }) + + t.Run("topology with network policies", func(t *testing.T) { + policy := &networkingv1.NetworkPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "NetworkPolicy", + APIVersion: "networking.k8s.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-policy", + Namespace: "test-ns", + }, + } + topology, err := machinery.NewTopology( + machinery.WithObjects(&controller.RuntimeObject{Object: policy}), + ) + assert.NilError(t, err) + + result := getNetworkPolicies(topology) + assert.Assert(t, is.Len(result, 1), "should return one policy") + assert.Equal(t, result[0].Name, "test-policy") + }) + + t.Run("multiple policies", func(t *testing.T) { + policy1 := &networkingv1.NetworkPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "NetworkPolicy", + APIVersion: "networking.k8s.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "policy1", + Namespace: "test-ns", + }, + } + policy2 := &networkingv1.NetworkPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "NetworkPolicy", + APIVersion: "networking.k8s.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "policy2", + Namespace: "test-ns", + }, + } + topology, err := machinery.NewTopology( + machinery.WithObjects( + &controller.RuntimeObject{Object: policy1}, + &controller.RuntimeObject{Object: policy2}, + ), + ) + assert.NilError(t, err) + + result := getNetworkPolicies(topology) + assert.Assert(t, is.Len(result, 2), "should return all policies") + }) + + t.Run("mixed objects - only NetworkPolicy kind returned", func(t *testing.T) { + policy := &networkingv1.NetworkPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "NetworkPolicy", + APIVersion: "networking.k8s.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-policy", + Namespace: "test-ns", + }, + } + gateway := &gatewayapiv1.Gateway{ + TypeMeta: metav1.TypeMeta{ + Kind: "Gateway", + APIVersion: "gateway.networking.k8s.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-gateway", + Namespace: "test-ns", + }, + } + topology, err := machinery.NewTopology( + machinery.WithObjects(&controller.RuntimeObject{Object: policy}), + machinery.WithTargetables(&machinery.Gateway{Gateway: gateway}), + ) + assert.NilError(t, err) + + result := getNetworkPolicies(topology) + assert.Assert(t, is.Len(result, 1), "should return only NetworkPolicy objects") + assert.Equal(t, result[0].Name, "test-policy") + }) +} + +func TestNewNetworkPolicyReconciler(t *testing.T) { + t.Run("constructor returns non-nil", func(t *testing.T) { + reconciler := NewNetworkPolicyReconciler(nil) + assert.Assert(t, reconciler != nil, "constructor should return non-nil reconciler") + }) + + t.Run("subscription has 9 events", func(t *testing.T) { + reconciler := NewNetworkPolicyReconciler(nil) + subscription := reconciler.Subscription() + + assert.Assert(t, subscription != nil, "subscription should not be nil") + assert.Assert(t, is.Len(subscription.Events, 9), "subscription should have 9 events") + }) + + t.Run("events match expected kinds and event types", func(t *testing.T) { + reconciler := NewNetworkPolicyReconciler(nil) + subscription := reconciler.Subscription() + events := subscription.Events + + // Kuadrant Create + assert.DeepEqual(t, events[0].Kind, &kuadrantv1beta1.KuadrantGroupKind) + assert.DeepEqual(t, events[0].EventType, ptr.To(controller.CreateEvent)) + + // Kuadrant Delete + assert.DeepEqual(t, events[1].Kind, &kuadrantv1beta1.KuadrantGroupKind) + assert.DeepEqual(t, events[1].EventType, ptr.To(controller.DeleteEvent)) + + // Authorino Create + assert.DeepEqual(t, events[2].Kind, &kuadrantv1beta1.AuthorinoGroupKind) + assert.DeepEqual(t, events[2].EventType, ptr.To(controller.CreateEvent)) + + // Authorino Update + assert.DeepEqual(t, events[3].Kind, &kuadrantv1beta1.AuthorinoGroupKind) + assert.DeepEqual(t, events[3].EventType, ptr.To(controller.UpdateEvent)) + + // Limitador Create + assert.DeepEqual(t, events[4].Kind, &kuadrantv1beta1.LimitadorGroupKind) + assert.DeepEqual(t, events[4].EventType, ptr.To(controller.CreateEvent)) + + // Limitador Update + assert.DeepEqual(t, events[5].Kind, &kuadrantv1beta1.LimitadorGroupKind) + assert.DeepEqual(t, events[5].EventType, ptr.To(controller.UpdateEvent)) + + // NetworkPolicy (all events) + assert.DeepEqual(t, events[6].Kind, &kuadrantv1beta1.NetworkPolicyGroupKind) + assert.Assert(t, events[6].EventType == nil, "NetworkPolicy should not have EventType specified") + + // Gateway Create + assert.DeepEqual(t, events[7].Kind, &machinery.GatewayGroupKind) + assert.DeepEqual(t, events[7].EventType, ptr.To(controller.CreateEvent)) + + // Gateway Delete + assert.DeepEqual(t, events[8].Kind, &machinery.GatewayGroupKind) + assert.DeepEqual(t, events[8].EventType, ptr.To(controller.DeleteEvent)) + }) +} diff --git a/internal/controller/state_of_the_world.go b/internal/controller/state_of_the_world.go index 006aef8ea..76dc1000d 100644 --- a/internal/controller/state_of_the_world.go +++ b/internal/controller/state_of_the_world.go @@ -28,6 +28,7 @@ import ( istioclientnetworkingv1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" @@ -157,6 +158,20 @@ func NewPolicyMachineryController(manager ctrlruntime.Manager, client *dynamic.D // labels propagation pattern would be more reliable as the kuadrant operator would be owning these labels controller.FilterResourcesByLabel[*appsv1.Deployment]("app=limitador"), )), + controller.WithRunnable("authorino deployment watcher", controller.Watch( + &appsv1.Deployment{}, + kuadrantv1beta1.DeploymentsResource, + metav1.NamespaceAll, + controller.WithPredicates(&ctrlruntimepredicate.TypedGenerationChangedPredicate[*appsv1.Deployment]{}), + controller.FilterResourcesByField[*appsv1.Deployment]("metadata.name=authorino"), + )), + controller.WithRunnable("networkPolicy watcher", controller.Watch( + &networkingv1.NetworkPolicy{}, + kuadrantv1beta1.NetworkPolicyResource, + metav1.NamespaceAll, + controller.WithPredicates(&ctrlruntimepredicate.TypedGenerationChangedPredicate[*networkingv1.NetworkPolicy]{}), + controller.FilterResourcesByLabel[*networkingv1.NetworkPolicy]("app.kubernetes.io/managed-by=kuadrant-operator"), + )), controller.WithPolicyKinds( kuadrantv1.DNSPolicyGroupKind, kuadrantv1.TLSPolicyGroupKind, @@ -168,6 +183,7 @@ func NewPolicyMachineryController(manager ctrlruntime.Manager, client *dynamic.D kuadrantv1beta1.KuadrantGroupKind, ConfigMapGroupKind, kuadrantv1beta1.DeploymentGroupKind, + kuadrantv1beta1.NetworkPolicyGroupKind, ), controller.WithObjectLinks( kuadrantv1beta1.LinkKuadrantToGatewayClasses, @@ -601,6 +617,7 @@ func (b *BootOptionsBuilder) getLimitadorOperatorOptions() ([]controller.Control controller.WithObjectLinks( kuadrantv1beta1.LinkKuadrantToLimitador, kuadrantv1beta1.LinkLimitadorToDeployment, + kuadrantv1beta1.LinkLimitadorToNetworkPolicy, ), ) @@ -644,6 +661,8 @@ func (b *BootOptionsBuilder) getAuthorinoOperatorOptions() ([]controller.Control ), controller.WithObjectLinks( kuadrantv1beta1.LinkKuadrantToAuthorino, + kuadrantv1beta1.LinkAuthorinoToDeployment, + kuadrantv1beta1.LinkAuthorinoToNetworkPolicy, authorino.LinkHTTPRouteRuleToAuthConfig, authorino.LinkGRPCRouteRuleToAuthConfig, ), @@ -812,6 +831,7 @@ func (b *BootOptionsBuilder) Reconciler() controller.ReconcileFunc { traceReconcileFunc("workflow.data_plane_policies", NewDataPlanePoliciesWorkflow(b.manager, b.client, b.isGatewayAPIInstalled, b.isIstioInstalled, b.isEnvoyGatewayInstalled, b.isLimitadorOperatorInstalled, b.isAuthorinoOperatorInstalled).Run), traceReconcileFunc("workflow.observability", NewObservabilityReconciler(b.client, b.manager, operatorNamespace).Subscription().Reconcile), traceReconcileFunc("workflow.developer_portal", NewDeveloperPortalReconciler(b.manager).Subscription().Reconcile), + traceReconcileFunc("workflow.networkpolicy", NewNetworkPolicyReconciler(b.client).Subscription().Reconcile), }, Postcondition: traceReconcileFunc("workflow.finalize", b.finalStepsWorkflow().Run), } From f0941fcef01f5a53b23e643e1e20bd1ec92ef001 Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Mon, 31 Aug 2026 11:57:02 +0100 Subject: [PATCH 2/2] UPDATE: address pr comments Signed-off-by: Jim Fitzpatrick --- ...adrant-operator.clusterserviceversion.yaml | 113 +++++++- .../controller/networkpolicy_reconciler.go | 163 ++++++++---- .../networkpolicy_reconciler_test.go | 250 ++++++++++++++++-- 3 files changed, 432 insertions(+), 94 deletions(-) diff --git a/bundle/manifests/kuadrant-operator.clusterserviceversion.yaml b/bundle/manifests/kuadrant-operator.clusterserviceversion.yaml index 5cadd4adc..fadf9ddb7 100644 --- a/bundle/manifests/kuadrant-operator.clusterserviceversion.yaml +++ b/bundle/manifests/kuadrant-operator.clusterserviceversion.yaml @@ -225,7 +225,7 @@ metadata: categories: Integration & Delivery console.openshift.io/plugins: '["kuadrant-console-plugin"]' containerImage: quay.io/kuadrant/kuadrant-operator:latest - createdAt: "2026-08-25T15:38:41Z" + createdAt: "2026-09-01T08:36:51Z" description: A Kubernetes Operator to manage the lifecycle of the Kuadrant system operators.operatorframework.io/builder: operator-sdk-v1.33.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v4 @@ -261,6 +261,9 @@ spec: kind: DNSPolicy name: dnspolicies.kuadrant.io version: v1 + - kind: KuadrantControlPlane + name: kuadrantcontrolplanes.kuadrant.io + version: v1alpha1 - description: Kuadrant configures installations of Kuadrant Service Protection components displayName: Kuadrant @@ -412,17 +415,39 @@ spec: - apiGroups: - "" resources: + - namespaces + verbs: + - get + - list + - watch + - apiGroups: + - "" + - events.k8s.io + resources: - events verbs: - create - patch - apiGroups: - - "" + - apiextensions.k8s.io resources: - - namespaces + - customresourcedefinitions + verbs: + - create + - list + - watch + - apiGroups: + - apiextensions.k8s.io + resourceNames: + - dnshealthcheckprobes.kuadrant.io + - dnsrecords.kuadrant.io + resources: + - customresourcedefinitions verbs: - get - list + - patch + - update - watch - apiGroups: - apps @@ -436,6 +461,12 @@ spec: - patch - update - watch + - apiGroups: + - authentication.k8s.io + resources: + - tokenreviews + verbs: + - create - apiGroups: - authorino.kuadrant.io resources: @@ -448,6 +479,12 @@ spec: - patch - update - watch + - apiGroups: + - authorization.k8s.io + resources: + - subjectaccessreviews + verbs: + - create - apiGroups: - cert-manager.io resources: @@ -605,6 +642,7 @@ spec: - kuadrant.io resources: - authpolicies + - kuadrantcontrolplanes verbs: - create - get @@ -628,6 +666,7 @@ spec: resources: - authpolicies/status - dnspolicies/status + - kuadrantcontrolplanes/status - kuadrants/status - ratelimitpolicies/status - tlspolicies/status @@ -729,6 +768,51 @@ spec: - patch - update - watch + - apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - dns-operator-manager-rolebinding + - dns-operator-remote-cluster-rolebinding + resources: + - clusterrolebindings + verbs: + - delete + - get + - patch + - update + - apiGroups: + - rbac.authorization.k8s.io + resources: + - clusterrolebindings + - clusterroles + verbs: + - create + - apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - dns-operator-manager-role + - dns-operator-remote-cluster-role + resources: + - clusterroles + verbs: + - bind + - escalate + - get + - patch + - update + - apiGroups: + - rbac.authorization.k8s.io + resources: + - rolebindings + - roles + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - security.istio.io resources: @@ -782,6 +866,8 @@ spec: value: quay.io/kuadrant/console-plugin:v0.6.0 - name: RELATED_IMAGE_CONSOLE_PLUGIN_PF5 value: quay.io/kuadrant/console-plugin:v0.1.5-2 + - name: RELATED_IMAGE_DNS_OPERATOR + value: quay.io/kuadrant/dns-operator:latest - name: OPERATOR_NAMESPACE valueFrom: fieldRef: @@ -864,14 +950,6 @@ spec: - patch serviceAccountName: developer-portal-controller-manager - rules: - - apiGroups: - - "" - resources: - - secrets - verbs: - - get - - list - - watch - apiGroups: - "" resources: @@ -903,6 +981,15 @@ spec: verbs: - create - patch + - apiGroups: + - operators.coreos.com + resources: + - subscriptions + - clusterserviceversions + verbs: + - get + - list + - delete serviceAccountName: kuadrant-operator-controller-manager strategy: deployment installModes: @@ -937,7 +1024,7 @@ spec: - email: didier@redhat.com name: Didier Di Cesare maturity: alpha - minKubeVersion: 1.19.0 + minKubeVersion: 1.25.0 provider: name: Red Hat url: https://github.com/Kuadrant/kuadrant-operator @@ -952,4 +1039,6 @@ spec: name: console-plugin-sdk1 - image: quay.io/kuadrant/console-plugin:v0.1.5-2 name: console-plugin-pf5 + - image: quay.io/kuadrant/dns-operator:latest + name: dns-operator version: 0.0.0 diff --git a/internal/controller/networkpolicy_reconciler.go b/internal/controller/networkpolicy_reconciler.go index 10e0d9edb..29500af43 100644 --- a/internal/controller/networkpolicy_reconciler.go +++ b/internal/controller/networkpolicy_reconciler.go @@ -2,7 +2,6 @@ package controllers import ( "context" - "errors" "fmt" "reflect" "slices" @@ -86,23 +85,29 @@ func (r *NetworkPolicyReconciler) Reconcile(ctx context.Context, _ []controller. var errs []error // ------------------------------------------------------------------------------------------------------------- span.AddEvent("setting authorino network policy") - update := false authorinoObj := GetAuthorinoFromTopology(topology, state) + exist, err := hasLinkedDeployment(&controller.RuntimeObject{Object: authorinoObj}, topology) + // err will be raised if the authorinoObj is nil, it is safe to assume from this poing if there is no error the authorinoObj is not nil + if err != nil { + // don't process network policies for deployments that don't exist + logger.V(1).Info("no found deployment", "err", err) + errs = append(errs, err) + } - minAuthorinoNetworkPolicy := generateAuthorinoNetworkPolicy(kObj, authorinoObj, topology) + if exist { + minAuthorinoNetworkPolicy := generateAuthorinoNetworkPolicy(kObj, authorinoObj, topology) - var existingAuthorinoNetworkPolicy *networkingv1.NetworkPolicy - for _, policy := range policies { - if policy.GetName() == AuthorinoNetworkPolicy { - existingAuthorinoNetworkPolicy = policy - break + var existingAuthorinoNetworkPolicy *networkingv1.NetworkPolicy + for _, policy := range policies { + if policy.GetName() == AuthorinoNetworkPolicy { + existingAuthorinoNetworkPolicy = policy + break + } } - } - desiredAuthorinoNetworkPolicy, update := mergeNetworkPolicy(*minAuthorinoNetworkPolicy, existingAuthorinoNetworkPolicy) + desiredAuthorinoNetworkPolicy, update := mergeNetworkPolicy(*minAuthorinoNetworkPolicy, existingAuthorinoNetworkPolicy) - if authorinoObj != nil { ownerRef := metav1.OwnerReference{ APIVersion: authorinoObj.GroupVersionKind().GroupVersion().String(), Kind: authorinoObj.Kind, @@ -124,15 +129,15 @@ func (r *NetworkPolicyReconciler) Reconcile(ctx context.Context, _ []controller. } desiredAuthorinoNetworkPolicy.SetOwnerReferences(existingOwnerRefs) - } - err := r.writePolicyToCluster(ctx, logger, span, desiredAuthorinoNetworkPolicy, writeChecks{ - Create: existingAuthorinoNetworkPolicy == nil, - Update: update, - }) - if err != nil { - logger.Error(err, "failed to write authorino network policy to cluster") - errs = append(errs, err) + err := r.writePolicyToCluster(ctx, logger, span, desiredAuthorinoNetworkPolicy, writeChecks{ + Create: existingAuthorinoNetworkPolicy == nil, + Update: update, + }) + if err != nil { + logger.Error(err, "failed to write authorino network policy to cluster") + errs = append(errs, err) + } } // ------------------------------------------------------------------------------------------------------------- @@ -140,19 +145,27 @@ func (r *NetworkPolicyReconciler) Reconcile(ctx context.Context, _ []controller. span.AddEvent("setting limitador network policy") lObj := GetLimitadorFromTopology(topology, state) - minLimitadorNetworkPolicy := generateLimitadorNetworkPolicy(kObj, lObj, topology) + exist, err = hasLinkedDeployment(&controller.RuntimeObject{Object: lObj}, topology) + // err will be raised if the lObj is nil, it is safe to assume from this poing if there is no error the lObj is not nil + if err != nil { + // don't process network policies for deployments that don't exist + logger.V(1).Info("no found deployment", "err", err) + errs = append(errs, err) + } + + if exist { + minLimitadorNetworkPolicy := generateLimitadorNetworkPolicy(kObj, lObj, topology) - var existingLimitadorNetworkPolicy *networkingv1.NetworkPolicy - for _, policy := range policies { - if policy.GetName() == LimitadorNetworkPolicy { - existingLimitadorNetworkPolicy = policy - break + var existingLimitadorNetworkPolicy *networkingv1.NetworkPolicy + for _, policy := range policies { + if policy.GetName() == LimitadorNetworkPolicy { + existingLimitadorNetworkPolicy = policy + break + } } - } - desiredLimitadorNetworkPolicy, update := mergeNetworkPolicy(*minLimitadorNetworkPolicy, existingLimitadorNetworkPolicy) + desiredLimitadorNetworkPolicy, update := mergeNetworkPolicy(*minLimitadorNetworkPolicy, existingLimitadorNetworkPolicy) - if lObj != nil { ownerRef := metav1.OwnerReference{ APIVersion: lObj.GroupVersionKind().GroupVersion().String(), Kind: lObj.Kind, @@ -174,24 +187,28 @@ func (r *NetworkPolicyReconciler) Reconcile(ctx context.Context, _ []controller. } desiredLimitadorNetworkPolicy.SetOwnerReferences(existingOwnerRefs) - } - err = r.writePolicyToCluster(ctx, logger, span, desiredLimitadorNetworkPolicy, writeChecks{ - Create: existingLimitadorNetworkPolicy == nil, - Update: update, - }) - if err != nil { - logger.Error(err, "failed to write limitador network policy to cluster") - errs = append(errs, err) + err = r.writePolicyToCluster(ctx, logger, span, desiredLimitadorNetworkPolicy, writeChecks{ + Create: existingLimitadorNetworkPolicy == nil, + Update: update, + }) + if err != nil { + logger.Error(err, "failed to write limitador network policy to cluster") + errs = append(errs, err) + } } // ------------------------------------------------------------------------------------------------------------- if len(errs) > 0 { span.SetStatus(codes.Error, "reconciliation completed with errors") + for _, err := range errs { + logger.Error(err, "reconciliation error") + } } else { span.SetStatus(codes.Ok, "") } - return errors.Join(errs...) + // Don't return errors as it can cancel the context of workflows running in parallel. + return nil } func (r *NetworkPolicyReconciler) writePolicyToCluster(ctx context.Context, logger logr.Logger, span trace.Span, networkPolicy *networkingv1.NetworkPolicy, check writeChecks) error { @@ -278,7 +295,7 @@ func generateAuthorinoNetworkPolicy(kObj *v1beta1.Kuadrant, aObj *authorinoopera // These default port values are hardcode into the authServerCmd in the authorino repo // https://github.com/Kuadrant/authorino/blob/58fecc6cdec38376fa7dba5638f1f7ecb6964cd0/main.go#L178-L218 gRPCport := 50051 - HTTPport := 5051 + HTTPport := 5001 OIDCdiscoveryPort := 8083 if aObj != nil { @@ -293,6 +310,18 @@ func generateAuthorinoNetworkPolicy(kObj *v1beta1.Kuadrant, aObj *authorinoopera } } + ingress := []networkingv1.NetworkPolicyIngressRule{ + // OIDC discovery endpoint + ingressRule([]networkingv1.NetworkPolicyPeer{}, OIDCdiscoveryPort), + } + + if len(fromNamespaces) > 0 { + // gRPC ext-auth from Envoy + ingress = append(ingress, ingressRule(fromNamespaces, gRPCport)) + // HTTP ext-auth from gateway + ingress = append(ingress, ingressRule(fromNamespaces, HTTPport)) + } + return &networkingv1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ Name: AuthorinoNetworkPolicy, @@ -304,14 +333,7 @@ func generateAuthorinoNetworkPolicy(kObj *v1beta1.Kuadrant, aObj *authorinoopera MatchLabels: labels, }, PolicyTypes: []networkingv1.PolicyType{"Ingress"}, - Ingress: []networkingv1.NetworkPolicyIngressRule{ - // gRPC ext-auth from Envoy - ingressRule(fromNamespaces, gRPCport), - // HTTP ext-auth from gateway - ingressRule(fromNamespaces, HTTPport), - // OIDC discovery endpoint - ingressRule([]networkingv1.NetworkPolicyPeer{}, OIDCdiscoveryPort), - }, + Ingress: ingress, }, } } @@ -365,10 +387,6 @@ func generateLimitadorNetworkPolicy(kObj *v1beta1.Kuadrant, lObj *limitadorv1alp labels := linkedDeploymentLabels(&controller.RuntimeObject{Object: lObj}, topology) - if labels == nil { - labels = map[string]string{"kuadrant.io/managed": "true"} - } - // These default port values are hardcode into the impl Configuration in the limitador repo // https://github.com/Kuadrant/limitador/blob/f73e5f4b3d9af3756d4d772b35d2798693b961f9/limitador-server/src/config.rs#L101-L103 gRPCport := 8081 @@ -379,6 +397,15 @@ func generateLimitadorNetworkPolicy(kObj *v1beta1.Kuadrant, lObj *limitadorv1alp HTTPport = int(lObj.HTTPPort()) } + ingress := []networkingv1.NetworkPolicyIngressRule{} + + if len(fromNamespaces) > 0 { + // gRPC ext-auth from Envoy + ingress = append(ingress, ingressRule(fromNamespaces, gRPCport)) + // HTTP ext-auth from gateway + ingress = append(ingress, ingressRule(fromNamespaces, HTTPport)) + } + return &networkingv1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ Name: LimitadorNetworkPolicy, @@ -390,12 +417,7 @@ func generateLimitadorNetworkPolicy(kObj *v1beta1.Kuadrant, lObj *limitadorv1alp MatchLabels: labels, }, PolicyTypes: []networkingv1.PolicyType{"Ingress"}, - Ingress: []networkingv1.NetworkPolicyIngressRule{ - // gRPC rate limit checks - ingressRule(fromNamespaces, gRPCport), - // HTTP rate limit checks - ingressRule(fromNamespaces, HTTPport), - }, + Ingress: ingress, }, } } @@ -430,3 +452,32 @@ func linkedDeploymentLabels(resource *controller.RuntimeObject, topology *machin return nil } + +func hasLinkedDeployment(resource *controller.RuntimeObject, topology *machinery.Topology) (bool, error) { + if resource == nil { + return false, fmt.Errorf("nil resource: *contronller.RuntimeObject provided") + } + + // Check for typed nil - when Object interface contains a nil pointer + v := reflect.ValueOf(resource.Object) + if !v.IsValid() || (v.Kind() == reflect.Pointer && v.IsNil()) { + return false, fmt.Errorf("provided resoucre has nil Object") + } + + deployments := lo.FilterMap(topology.All().Children(resource), func(child machinery.Object, _ int) (machinery.Object, bool) { + if child.GroupVersionKind().GroupKind() != v1beta1.DeploymentGroupKind { + return nil, false + } + return child, true + }) + + if len(deployments) == 1 { + return true, nil + } + + if len(deployments) > 1 { + return false, fmt.Errorf("more than one attached deployment found") + } + + return false, nil +} diff --git a/internal/controller/networkpolicy_reconciler_test.go b/internal/controller/networkpolicy_reconciler_test.go index 2d7dbf658..eb28f6b30 100644 --- a/internal/controller/networkpolicy_reconciler_test.go +++ b/internal/controller/networkpolicy_reconciler_test.go @@ -371,7 +371,7 @@ func TestGenerateAuthorinoNetworkPolicy(t *testing.T) { }, } - t.Run("nil authorino - default ports", func(t *testing.T) { + t.Run("nil authorino no gateways - only OIDC rule", func(t *testing.T) { topology, err := machinery.NewTopology() assert.NilError(t, err) @@ -380,17 +380,45 @@ func TestGenerateAuthorinoNetworkPolicy(t *testing.T) { assert.Equal(t, result.Name, AuthorinoNetworkPolicy) assert.Equal(t, result.Namespace, "kuadrant-system") assert.DeepEqual(t, result.Spec.PodSelector.MatchLabels, map[string]string{"kuadrant.io/managed": "true"}) - assert.Assert(t, is.Len(result.Spec.Ingress, 3), "should have 3 ingress rules") + assert.Assert(t, is.Len(result.Spec.Ingress, 1), "should have only OIDC ingress rule when no gateways") - // Verify gRPC port (default 50051) - assert.DeepEqual(t, result.Spec.Ingress[0].Ports[0].Port, new(intstr.FromInt(50051))) - // Verify HTTP port (default 5051) - assert.DeepEqual(t, result.Spec.Ingress[1].Ports[0].Port, new(intstr.FromInt(5051))) // Verify OIDC port (default 8083) - assert.DeepEqual(t, result.Spec.Ingress[2].Ports[0].Port, new(intstr.FromInt(8083))) + assert.DeepEqual(t, result.Spec.Ingress[0].Ports[0].Port, new(intstr.FromInt(8083))) + assert.Assert(t, is.Len(result.Spec.Ingress[0].From, 0), "OIDC should not have peers") + }) + + t.Run("nil authorino with gateways - default ports", func(t *testing.T) { + gateway := &gatewayapiv1.Gateway{ + TypeMeta: metav1.TypeMeta{ + Kind: "Gateway", + APIVersion: "gateway.networking.k8s.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "gw1", + Namespace: "gateway-ns", + }, + } + topology, err := machinery.NewTopology( + machinery.WithTargetables(&machinery.Gateway{Gateway: gateway}), + ) + assert.NilError(t, err) + + result := generateAuthorinoNetworkPolicy(kuadrant, nil, topology) + + assert.Assert(t, is.Len(result.Spec.Ingress, 3), "should have 3 ingress rules when gateways exist") + + // Verify OIDC port (default 8083) - first rule, no peers + assert.DeepEqual(t, result.Spec.Ingress[0].Ports[0].Port, new(intstr.FromInt(8083))) + assert.Assert(t, is.Len(result.Spec.Ingress[0].From, 0), "OIDC should not have peers") + // Verify gRPC port (default 50051) + assert.DeepEqual(t, result.Spec.Ingress[1].Ports[0].Port, new(intstr.FromInt(50051))) + assert.Assert(t, is.Len(result.Spec.Ingress[1].From, 1), "gRPC should have gateway peer") + // Verify HTTP port (default 5001) + assert.DeepEqual(t, result.Spec.Ingress[2].Ports[0].Port, new(intstr.FromInt(5001))) + assert.Assert(t, is.Len(result.Spec.Ingress[2].From, 1), "HTTP should have gateway peer") }) - t.Run("authorino with custom ports", func(t *testing.T) { + t.Run("authorino with custom ports and gateways", func(t *testing.T) { authorino := &authorinooperatorv1beta1.Authorino{ TypeMeta: metav1.TypeMeta{ Kind: "Authorino", @@ -412,17 +440,61 @@ func TestGenerateAuthorinoNetworkPolicy(t *testing.T) { }, }, } - topology, err := machinery.NewTopology() + gateway := &gatewayapiv1.Gateway{ + TypeMeta: metav1.TypeMeta{ + Kind: "Gateway", + APIVersion: "gateway.networking.k8s.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "gw1", + Namespace: "gateway-ns", + }, + } + topology, err := machinery.NewTopology( + machinery.WithTargetables(&machinery.Gateway{Gateway: gateway}), + ) assert.NilError(t, err) result := generateAuthorinoNetworkPolicy(kuadrant, authorino, topology) + assert.Assert(t, is.Len(result.Spec.Ingress, 3), "should have 3 ingress rules with gateways") + // Verify custom OIDC port (first rule) + assert.DeepEqual(t, result.Spec.Ingress[0].Ports[0].Port, new(intstr.FromInt(9002))) // Verify custom gRPC port - assert.DeepEqual(t, result.Spec.Ingress[0].Ports[0].Port, new(intstr.FromInt(9000))) + assert.DeepEqual(t, result.Spec.Ingress[1].Ports[0].Port, new(intstr.FromInt(9000))) // Verify custom HTTP port - assert.DeepEqual(t, result.Spec.Ingress[1].Ports[0].Port, new(intstr.FromInt(9001))) - // Verify custom OIDC port - assert.DeepEqual(t, result.Spec.Ingress[2].Ports[0].Port, new(intstr.FromInt(9002))) + assert.DeepEqual(t, result.Spec.Ingress[2].Ports[0].Port, new(intstr.FromInt(9001))) + }) + + t.Run("authorino with custom ports no gateways - only OIDC", func(t *testing.T) { + authorino := &authorinooperatorv1beta1.Authorino{ + TypeMeta: metav1.TypeMeta{ + Kind: "Authorino", + APIVersion: "operator.authorino.kuadrant.io/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "authorino", + Namespace: "kuadrant-system", + }, + Spec: authorinooperatorv1beta1.AuthorinoSpec{ + Listener: authorinooperatorv1beta1.Listener{ + Ports: authorinooperatorv1beta1.Ports{ + GRPC: new(int32(9000)), + HTTP: new(int32(9001)), + }, + }, + OIDCServer: authorinooperatorv1beta1.OIDCServer{ + Port: new(int32(9002)), + }, + }, + } + topology, err := machinery.NewTopology() + assert.NilError(t, err) + + result := generateAuthorinoNetworkPolicy(kuadrant, authorino, topology) + + assert.Assert(t, is.Len(result.Spec.Ingress, 1), "should have only OIDC rule without gateways") + assert.DeepEqual(t, result.Spec.Ingress[0].Ports[0].Port, new(intstr.FromInt(9002))) }) t.Run("gateway in topology - peers in gRPC and HTTP but not OIDC", func(t *testing.T) { @@ -443,18 +515,20 @@ func TestGenerateAuthorinoNetworkPolicy(t *testing.T) { result := generateAuthorinoNetworkPolicy(kuadrant, nil, topology) - // gRPC rule should have gateway peer - assert.Assert(t, is.Len(result.Spec.Ingress[0].From, 1), "gRPC should have gateway peer") - assert.DeepEqual(t, result.Spec.Ingress[0].From[0].NamespaceSelector.MatchLabels, - map[string]string{"kubernetes.io/metadata.name": "gateway-ns"}) + assert.Assert(t, is.Len(result.Spec.Ingress, 3), "should have 3 ingress rules") - // HTTP rule should have gateway peer - assert.Assert(t, is.Len(result.Spec.Ingress[1].From, 1), "HTTP should have gateway peer") + // OIDC rule (index 0) should NOT have gateway peer (empty From) + assert.Assert(t, is.Len(result.Spec.Ingress[0].From, 0), "OIDC should not have gateway peer") + + // gRPC rule (index 1) should have gateway peer + assert.Assert(t, is.Len(result.Spec.Ingress[1].From, 1), "gRPC should have gateway peer") assert.DeepEqual(t, result.Spec.Ingress[1].From[0].NamespaceSelector.MatchLabels, map[string]string{"kubernetes.io/metadata.name": "gateway-ns"}) - // OIDC rule should NOT have gateway peer (empty From) - assert.Assert(t, is.Len(result.Spec.Ingress[2].From, 0), "OIDC should not have gateway peer") + // HTTP rule (index 2) should have gateway peer + assert.Assert(t, is.Len(result.Spec.Ingress[2].From, 1), "HTTP should have gateway peer") + assert.DeepEqual(t, result.Spec.Ingress[2].From[0].NamespaceSelector.MatchLabels, + map[string]string{"kubernetes.io/metadata.name": "gateway-ns"}) }) t.Run("common labels are set", func(t *testing.T) { @@ -482,7 +556,7 @@ func TestGenerateLimitadorNetworkPolicy(t *testing.T) { }, } - t.Run("nil limitador - default ports", func(t *testing.T) { + t.Run("nil limitador no gateways - empty ingress", func(t *testing.T) { topology, err := machinery.NewTopology() assert.NilError(t, err) @@ -490,16 +564,36 @@ func TestGenerateLimitadorNetworkPolicy(t *testing.T) { assert.Equal(t, result.Name, LimitadorNetworkPolicy) assert.Equal(t, result.Namespace, "kuadrant-system") - assert.DeepEqual(t, result.Spec.PodSelector.MatchLabels, map[string]string{"kuadrant.io/managed": "true"}) - assert.Assert(t, is.Len(result.Spec.Ingress, 2), "should have 2 ingress rules") + assert.Assert(t, result.Spec.PodSelector.MatchLabels == nil, "should have nil labels when no deployment linked") + assert.Assert(t, is.Len(result.Spec.Ingress, 0), "should have no ingress rules without gateways") + }) + + t.Run("nil limitador with gateways - default ports", func(t *testing.T) { + gateway := &gatewayapiv1.Gateway{ + TypeMeta: metav1.TypeMeta{ + Kind: "Gateway", + APIVersion: "gateway.networking.k8s.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "gw1", + Namespace: "gateway-ns", + }, + } + topology, err := machinery.NewTopology( + machinery.WithTargetables(&machinery.Gateway{Gateway: gateway}), + ) + assert.NilError(t, err) + + result := generateLimitadorNetworkPolicy(kuadrant, nil, topology) + assert.Assert(t, is.Len(result.Spec.Ingress, 2), "should have 2 ingress rules with gateways") // Verify gRPC port (default 8081) assert.DeepEqual(t, result.Spec.Ingress[0].Ports[0].Port, new(intstr.FromInt(8081))) // Verify HTTP port (default 8080) assert.DeepEqual(t, result.Spec.Ingress[1].Ports[0].Port, new(intstr.FromInt(8080))) }) - t.Run("limitador with custom ports", func(t *testing.T) { + t.Run("limitador with custom ports and gateways", func(t *testing.T) { limitador := &limitadorv1alpha1.Limitador{ TypeMeta: metav1.TypeMeta{ Kind: "Limitador", @@ -516,11 +610,24 @@ func TestGenerateLimitadorNetworkPolicy(t *testing.T) { }, }, } - topology, err := machinery.NewTopology() + gateway := &gatewayapiv1.Gateway{ + TypeMeta: metav1.TypeMeta{ + Kind: "Gateway", + APIVersion: "gateway.networking.k8s.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "gw1", + Namespace: "gateway-ns", + }, + } + topology, err := machinery.NewTopology( + machinery.WithTargetables(&machinery.Gateway{Gateway: gateway}), + ) assert.NilError(t, err) result := generateLimitadorNetworkPolicy(kuadrant, limitador, topology) + assert.Assert(t, is.Len(result.Spec.Ingress, 2), "should have 2 ingress rules with gateways") // Verify custom gRPC port assert.DeepEqual(t, result.Spec.Ingress[0].Ports[0].Port, new(intstr.FromInt(7001))) // Verify custom HTTP port @@ -662,6 +769,97 @@ func TestLinkedDeploymentLabels(t *testing.T) { }) } +func TestHasLinkedDeployment(t *testing.T) { + t.Run("nil resource", func(t *testing.T) { + topology, err := machinery.NewTopology() + assert.NilError(t, err) + + exist, err := hasLinkedDeployment(nil, topology) + assert.Assert(t, !exist, "should return false for nil resource") + assert.ErrorContains(t, err, "nil resource") + }) + + t.Run("typed nil Object", func(t *testing.T) { + topology, err := machinery.NewTopology() + assert.NilError(t, err) + + var nilAuthorino *authorinooperatorv1beta1.Authorino + resource := &controller.RuntimeObject{Object: nilAuthorino} + + exist, err := hasLinkedDeployment(resource, topology) + assert.Assert(t, !exist, "should return false for typed nil Object") + assert.ErrorContains(t, err, "nil Object") + }) + + t.Run("no deployment children", func(t *testing.T) { + authorino := &authorinooperatorv1beta1.Authorino{ + TypeMeta: metav1.TypeMeta{ + Kind: "Authorino", + APIVersion: "operator.authorino.kuadrant.io/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "authorino", + Namespace: "kuadrant-system", + UID: "authorino-uid", + }, + } + authorinoRuntimeObj := &controller.RuntimeObject{Object: authorino} + + topology, err := machinery.NewTopology( + machinery.WithObjects(authorinoRuntimeObj), + ) + assert.NilError(t, err) + + exist, err := hasLinkedDeployment(authorinoRuntimeObj, topology) + assert.Assert(t, !exist, "should return false when no deployment children") + assert.NilError(t, err, "should not return error when no deployment children") + }) + + t.Run("one deployment child", func(t *testing.T) { + authorino := &authorinooperatorv1beta1.Authorino{ + TypeMeta: metav1.TypeMeta{ + Kind: "Authorino", + APIVersion: "operator.authorino.kuadrant.io/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "authorino", + Namespace: "kuadrant-system", + UID: "authorino-uid", + }, + } + deployment := &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: "Deployment", + APIVersion: "apps/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "authorino", + Namespace: "kuadrant-system", + UID: "deployment-uid", + }, + } + + authorinoRuntimeObj := &controller.RuntimeObject{Object: authorino} + deploymentRuntimeObj := &controller.RuntimeObject{Object: deployment} + + store := make(controller.Store) + store[string(authorinoRuntimeObj.GetUID())] = authorinoRuntimeObj + store[string(deploymentRuntimeObj.GetUID())] = deploymentRuntimeObj + + linkFunc := kuadrantv1beta1.LinkAuthorinoToDeployment(store) + + topology, err := machinery.NewTopology( + machinery.WithObjects(authorinoRuntimeObj, deploymentRuntimeObj), + machinery.WithLinks(linkFunc), + ) + assert.NilError(t, err) + + exist, err := hasLinkedDeployment(authorinoRuntimeObj, topology) + assert.Assert(t, exist, "should return true for one deployment child") + assert.NilError(t, err) + }) +} + func TestGetNetworkPolicies(t *testing.T) { t.Run("empty topology", func(t *testing.T) { topology, err := machinery.NewTopology()