From 219ee9a0b6befbf7713c0a32b4ee80f6f37cba64 Mon Sep 17 00:00:00 2001 From: "M. Essam" Date: Fri, 28 Mar 2025 14:50:09 +0200 Subject: [PATCH] Add support for multiple policies (#16) Change policy annotation to support comma-separated list of policies. --------- Co-authored-by: Maycon Santos --- docs/usage.md | 5 +- helm/kubernetes-operator/secretvalues.yaml | 16 -- internal/controller/nbpolicy_controller.go | 2 +- internal/controller/nbresource_controller.go | 176 ++++++++++-------- .../controller/nbresource_controller_test.go | 129 +++++++++++++ internal/util/slices.go | 15 ++ 6 files changed, 250 insertions(+), 93 deletions(-) delete mode 100644 helm/kubernetes-operator/secretvalues.yaml diff --git a/docs/usage.md b/docs/usage.md index 5cca9c6..222310f 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -86,7 +86,7 @@ cluster: |`netbird.io/expose`| Expose service using NetBird Network Resource ||(`null`, `true`)| |`netbird.io/groups`| Comma-separated list of group names to assign to Network Resource |`{ClusterName}-{Namespace}-{Service}`|Any comma-separated list of strings.| |`netbird.io/resource-name`| Network Resource name |`{Namespace}-{Service}`|Any valid network resource name, make sure they're unique!| -|`netbird.io/policy`| Name of NBPolicy to propagate service ports as destination. ||Name of any NBPolicy resource| +|`netbird.io/policy`| Name(s) of NBPolicy to propagate service ports as destination. ||Comma-separated list of names of any NBPolicy resource| |`netbird.io/policy-ports`| Narrow down exposed ports in a policy. Leave empty for all ports. ||Comma-separated integer list, integers must be between 0-65535| |`netbird.io/policy-protocol`| Narrow down protocol for use in a policy. Leave empty for all protocols. ||(`tcp`,`udp`)| @@ -158,7 +158,7 @@ ingress: - udp bidirectional: true # Optional, defaults to true ``` -2. Reference policy in Services using `netbird.io/policy=default`, this will add relevant ports and destination groups to policy. +2. Reference policies in Services using `netbird.io/policy=default,otherpolicy,...`, this will add relevant ports and destination groups to policies. 3. (Optional) Limit specific ports in exposed service by adding `netbird.io/policy-ports=443`. 4. (Optional) Limit specific protocol in exposed service by adding `netbird.io/policy-protocol=tcp`. @@ -166,3 +166,4 @@ ingress: * Each NBPolicy will only create policies in the NetBird console when the information provided is enough to create one. If no services act as a destination or specified services do not conform to the protocol(s) defined, the policy will not be created. * Each NBPolicy will create one policy in the NetBird console per protocol specified as long as the protocol has destinations; this ensures better-secured policies by separating ports for TCP and UDP. * Policies currently do not support ICMP protocol, as ICMP is not supported in Kubernetes services, and there are [no current plans to support it](https://discuss.kubernetes.io/t/icmp-support-for-kubernetes-service/21738). +* NetBird currently does not support SCTP protocol. \ No newline at end of file diff --git a/helm/kubernetes-operator/secretvalues.yaml b/helm/kubernetes-operator/secretvalues.yaml deleted file mode 100644 index ae050bb..0000000 --- a/helm/kubernetes-operator/secretvalues.yaml +++ /dev/null @@ -1,16 +0,0 @@ -netbirdAPI: - key: "nbp_1TpgsCUaF2BU3NVZfaDrGpoY1jx2iT2g7rWj" - -operator: - image: - tag: "v0.1.0" - -ingress: - enabled: true - router: - enabled: true - policies: - default: - name: Kubernetes Default Policy - sourceGroups: - - All diff --git a/internal/controller/nbpolicy_controller.go b/internal/controller/nbpolicy_controller.go index 9c41479..c784d56 100644 --- a/internal/controller/nbpolicy_controller.go +++ b/internal/controller/nbpolicy_controller.go @@ -77,7 +77,7 @@ func (r *NBPolicyReconciler) mapResources(ctx context.Context, nbPolicy *netbird } for _, resource := range resources { - if resource.Status.PolicyName != nil && *resource.Status.PolicyName == nbPolicy.Name { + if resource.Status.PolicyName != nil && util.Contains(util.SplitTrim(*resource.Status.PolicyName, ","), nbPolicy.Name) { // Groups groups = append(groups, resource.Status.Groups...) diff --git a/internal/controller/nbresource_controller.go b/internal/controller/nbresource_controller.go index 2e42000..78b0244 100644 --- a/internal/controller/nbresource_controller.go +++ b/internal/controller/nbresource_controller.go @@ -109,82 +109,108 @@ func (r *NBResourceReconciler) handlePolicy(ctx context.Context, req ctrl.Reques return nil } - updatePolicyStatus := false - var nbPolicy netbirdiov1.NBPolicy if nbResource.Spec.PolicyName == "" && nbResource.Status.PolicyName != nil { // Remove self reference from policy status - err := r.Client.Get(ctx, types.NamespacedName{Name: *nbResource.Status.PolicyName}, &nbPolicy) - nbResource.Status.PolicyName = nil - if err != nil { - logger.Error(errKubernetesAPI, "error getting NBPolicy", "err", err, "policyName", nbResource.Spec.PolicyName) - return err - } - if util.Contains(nbPolicy.Status.ManagedServiceList, req.NamespacedName.String()) { - nbPolicy.Status.ManagedServiceList = util.Without(nbPolicy.Status.ManagedServiceList, req.NamespacedName.String()) - nbPolicy.Status.LastUpdatedAt = &v1.Time{Time: time.Now()} - updatePolicyStatus = true - } - } else { - // Update policy settings if any difference is found - if nbResource.Status.PolicyName != nil { - err := r.Client.Get(ctx, types.NamespacedName{Name: *nbResource.Status.PolicyName}, &nbPolicy) - if !errors.IsNotFound(err) { + policies := util.SplitTrim(*nbResource.Status.PolicyName, ",") + for _, policyName := range policies { + err := r.Client.Get(ctx, types.NamespacedName{Name: policyName}, &nbPolicy) + nbResource.Status.PolicyName = nil + if err != nil { + logger.Error(errKubernetesAPI, "error getting NBPolicy", "err", err, "policyName", policyName) + return err + } + if util.Contains(nbPolicy.Status.ManagedServiceList, req.NamespacedName.String()) { + nbPolicy.Status.ManagedServiceList = util.Without(nbPolicy.Status.ManagedServiceList, req.NamespacedName.String()) + nbPolicy.Status.LastUpdatedAt = &v1.Time{Time: time.Now()} + err := r.Client.Status().Update(ctx, &nbPolicy) if err != nil { - logger.Error(errKubernetesAPI, "error getting NBPolicy", "err", err, "policyName", nbResource.Spec.PolicyName) + logger.Error(errKubernetesAPI, "error updating NBPolicy", "err", err, "policyName", policyName) return err } + } + } + } else { + specPolicies := util.SplitTrim(nbResource.Spec.PolicyName, ",") + var statusPolicies []string + if nbResource.Status.PolicyName != nil { + statusPolicies = util.SplitTrim(*nbResource.Status.PolicyName, ",") + } - if util.Contains(nbPolicy.Status.ManagedServiceList, req.NamespacedName.String()) { - nbPolicy.Status.ManagedServiceList = util.Without(nbPolicy.Status.ManagedServiceList, req.NamespacedName.String()) - err := r.Client.Status().Update(ctx, &nbPolicy) + for _, policy := range specPolicies { + updatePolicyStatus := false + + err := r.Client.Get(ctx, types.NamespacedName{Name: policy}, &nbPolicy) + if err != nil { + logger.Error(errKubernetesAPI, "error getting NBPolicy", "err", err, "policyName", policy) + return err + } + if !util.Contains(statusPolicies, policy) { + // New + if !util.Contains(nbPolicy.Status.ManagedServiceList, req.NamespacedName.String()) { + nbPolicy.Status.ManagedServiceList = append(nbPolicy.Status.ManagedServiceList, req.NamespacedName.String()) + updatePolicyStatus = true + } + } else { + // Check update + if !util.Contains(nbPolicy.Status.ManagedServiceList, req.NamespacedName.String()) { + nbPolicy.Status.ManagedServiceList = append(nbPolicy.Status.ManagedServiceList, req.NamespacedName.String()) + nbPolicy.Status.LastUpdatedAt = &v1.Time{Time: time.Now()} + updatePolicyStatus = true + } + + if !util.Equivalent(nbResource.Spec.TCPPorts, nbResource.Status.TCPPorts) { + nbResource.Status.TCPPorts = nbResource.Spec.TCPPorts + nbPolicy.Status.LastUpdatedAt = &v1.Time{Time: time.Now()} + updatePolicyStatus = true + } + + if !util.Equivalent(nbResource.Spec.UDPPorts, nbResource.Status.UDPPorts) { + nbResource.Status.UDPPorts = nbResource.Spec.UDPPorts + nbPolicy.Status.LastUpdatedAt = &v1.Time{Time: time.Now()} + updatePolicyStatus = true + } + + if !util.Equivalent(nbResource.Status.Groups, groupIDs) { + nbResource.Status.Groups = groupIDs + nbPolicy.Status.LastUpdatedAt = &v1.Time{Time: time.Now()} + updatePolicyStatus = true + } + } + + if updatePolicyStatus { + err := r.Client.Status().Update(ctx, &nbPolicy) + if err != nil { + logger.Error(errKubernetesAPI, "error updating NBPolicy", "err", err, "policyName", policy) + return err + } + } + } + + for _, policy := range statusPolicies { + // Delete + if !util.Contains(specPolicies, policy) { + err := r.Client.Get(ctx, types.NamespacedName{Name: policy}, &nbPolicy) + if !errors.IsNotFound(err) { if err != nil { - logger.Error(errKubernetesAPI, "error updating NBPolicy", "err", err, "policyName", nbResource.Spec.PolicyName) + logger.Error(errKubernetesAPI, "error getting NBPolicy", "err", err, "policyName", policy) return err } + + if util.Contains(nbPolicy.Status.ManagedServiceList, req.NamespacedName.String()) { + nbPolicy.Status.ManagedServiceList = util.Without(nbPolicy.Status.ManagedServiceList, req.NamespacedName.String()) + err := r.Client.Status().Update(ctx, &nbPolicy) + if err != nil { + logger.Error(errKubernetesAPI, "error updating NBPolicy", "err", err, "policyName", policy) + return err + } + } } } } - err := r.Client.Get(ctx, types.NamespacedName{Name: nbResource.Spec.PolicyName}, &nbPolicy) - if err != nil { - logger.Error(errKubernetesAPI, "error getting NBPolicy", "err", err, "policyName", nbResource.Spec.PolicyName) - return err - } - if nbResource.Status.PolicyName == nil || *nbResource.Status.PolicyName != nbPolicy.Name { - nbResource.Status.PolicyName = &nbPolicy.Name - } - - if !util.Contains(nbPolicy.Status.ManagedServiceList, req.NamespacedName.String()) { - nbPolicy.Status.ManagedServiceList = append(nbPolicy.Status.ManagedServiceList, req.NamespacedName.String()) - nbPolicy.Status.LastUpdatedAt = &v1.Time{Time: time.Now()} - updatePolicyStatus = true - } - - if !util.Equivalent(nbResource.Spec.TCPPorts, nbResource.Status.TCPPorts) { - nbResource.Status.TCPPorts = nbResource.Spec.TCPPorts - nbPolicy.Status.LastUpdatedAt = &v1.Time{Time: time.Now()} - updatePolicyStatus = true - } - - if !util.Equivalent(nbResource.Spec.UDPPorts, nbResource.Status.UDPPorts) { - nbResource.Status.UDPPorts = nbResource.Spec.UDPPorts - nbPolicy.Status.LastUpdatedAt = &v1.Time{Time: time.Now()} - updatePolicyStatus = true - } - - if !util.Equivalent(nbResource.Status.Groups, groupIDs) { - nbResource.Status.Groups = groupIDs - nbPolicy.Status.LastUpdatedAt = &v1.Time{Time: time.Now()} - updatePolicyStatus = true - } - } - - if updatePolicyStatus { - err := r.Client.Status().Update(ctx, &nbPolicy) - if err != nil { - logger.Error(errKubernetesAPI, "error updating NBPolicy", "err", err, "policyName", nbResource.Spec.PolicyName) - return err + if nbResource.Status.PolicyName == nil || *nbResource.Status.PolicyName != nbResource.Spec.PolicyName { + nbResource.Status.PolicyName = &nbResource.Spec.PolicyName } } @@ -402,20 +428,22 @@ func (r *NBResourceReconciler) handleGroups(ctx context.Context, req ctrl.Reques func (r *NBResourceReconciler) handleDelete(ctx context.Context, req ctrl.Request, nbResource *netbirdiov1.NBResource, logger logr.Logger) error { if nbResource.Status.PolicyName != nil { - var nbPolicy netbirdiov1.NBPolicy - err := r.Client.Get(ctx, types.NamespacedName{Name: *nbResource.Status.PolicyName}, &nbPolicy) - if err != nil && !errors.IsNotFound(err) { - logger.Error(errKubernetesAPI, "error getting NBPolicy", "err", err, "policyName", nbResource.Spec.PolicyName) - return err - } - - if !errors.IsNotFound(err) && util.Contains(nbPolicy.Status.ManagedServiceList, req.NamespacedName.String()) { - nbPolicy.Status.ManagedServiceList = util.Without(nbPolicy.Status.ManagedServiceList, req.NamespacedName.String()) - nbPolicy.Status.LastUpdatedAt = &v1.Time{Time: time.Now()} - err = r.Client.Status().Update(ctx, &nbPolicy) - if err != nil { + for _, policy := range util.SplitTrim(*nbResource.Status.PolicyName, ",") { + var nbPolicy netbirdiov1.NBPolicy + err := r.Client.Get(ctx, types.NamespacedName{Name: policy}, &nbPolicy) + if err != nil && !errors.IsNotFound(err) { + logger.Error(errKubernetesAPI, "error getting NBPolicy", "err", err, "policyName", policy) return err } + + if !errors.IsNotFound(err) && util.Contains(nbPolicy.Status.ManagedServiceList, req.NamespacedName.String()) { + nbPolicy.Status.ManagedServiceList = util.Without(nbPolicy.Status.ManagedServiceList, req.NamespacedName.String()) + nbPolicy.Status.LastUpdatedAt = &v1.Time{Time: time.Now()} + err = r.Client.Status().Update(ctx, &nbPolicy) + if err != nil { + return err + } + } } } diff --git a/internal/controller/nbresource_controller_test.go b/internal/controller/nbresource_controller_test.go index 8e265b6..916a95e 100644 --- a/internal/controller/nbresource_controller_test.go +++ b/internal/controller/nbresource_controller_test.go @@ -414,6 +414,135 @@ var _ = Describe("NBResource Controller", func() { }) }) + When("Multiple Policies are specified", Ordered, func() { + BeforeAll(func() { + nbPolicy := &netbirdiov1.NBPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-a", + }, + Spec: netbirdiov1.NBPolicySpec{ + Name: "Test A", + SourceGroups: []string{"All"}, + }, + } + Expect(k8sClient.Create(ctx, nbPolicy)).To(Succeed()) + + nbPolicy = &netbirdiov1.NBPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-b", + }, + Spec: netbirdiov1.NBPolicySpec{ + Name: "Test B", + SourceGroups: []string{"All"}, + }, + } + Expect(k8sClient.Create(ctx, nbPolicy)).To(Succeed()) + + nbPolicy = &netbirdiov1.NBPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-c", + }, + Spec: netbirdiov1.NBPolicySpec{ + Name: "Test C", + SourceGroups: []string{"All"}, + }, + } + Expect(k8sClient.Create(ctx, nbPolicy)).To(Succeed()) + }) + + AfterAll(func() { + nbPolicy := &netbirdiov1.NBPolicy{} + err := k8sClient.Get(ctx, types.NamespacedName{Name: "test-a"}, nbPolicy) + if !errors.IsNotFound(err) { + Expect(k8sClient.Delete(ctx, nbPolicy)).To(Succeed()) + } + + nbPolicy = &netbirdiov1.NBPolicy{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: "test-b"}, nbPolicy) + if !errors.IsNotFound(err) { + Expect(k8sClient.Delete(ctx, nbPolicy)).To(Succeed()) + } + + nbPolicy = &netbirdiov1.NBPolicy{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: "test-c"}, nbPolicy) + if !errors.IsNotFound(err) { + Expect(k8sClient.Delete(ctx, nbPolicy)).To(Succeed()) + } + }) + + It("should update policies status", func() { + nbresource.Spec.PolicyName = "test-a, test-b" + Expect(k8sClient.Update(ctx, nbresource)).To(Succeed()) + + _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + nbPolicy := &netbirdiov1.NBPolicy{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "test-a"}, nbPolicy)).To(Succeed()) + Expect(nbPolicy.Status.ManagedServiceList).To(ContainElement("default/test-resource")) + + nbPolicy = &netbirdiov1.NBPolicy{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "test-b"}, nbPolicy)).To(Succeed()) + Expect(nbPolicy.Status.ManagedServiceList).To(ContainElement("default/test-resource")) + }) + + When("Policy is updated", func() { + It("should remove old reference and add new reference", func() { + nbresource.Spec.PolicyName = "test-b,test-c" + Expect(k8sClient.Update(ctx, nbresource)).To(Succeed()) + + nbresource.Status.PolicyName = util.Ptr("test-a,test-b") + Expect(k8sClient.Status().Update(ctx, nbresource)).To(Succeed()) + + _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + nbPolicy := &netbirdiov1.NBPolicy{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "test-a"}, nbPolicy)).To(Succeed()) + Expect(nbPolicy.Status.ManagedServiceList).NotTo(ContainElement("default/test-resource")) + + nbPolicy = &netbirdiov1.NBPolicy{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "test-b"}, nbPolicy)).To(Succeed()) + Expect(nbPolicy.Status.ManagedServiceList).To(ContainElement("default/test-resource")) + + nbPolicy = &netbirdiov1.NBPolicy{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "test-c"}, nbPolicy)).To(Succeed()) + Expect(nbPolicy.Status.ManagedServiceList).To(ContainElement("default/test-resource")) + }) + }) + + When("Policy is removed", func() { + It("should remove old reference", func() { + nbresource.Spec.PolicyName = "" + Expect(k8sClient.Update(ctx, nbresource)).To(Succeed()) + + nbresource.Status.PolicyName = util.Ptr("test-b,test-c") + Expect(k8sClient.Status().Update(ctx, nbresource)).To(Succeed()) + + _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + nbPolicy := &netbirdiov1.NBPolicy{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "test-a"}, nbPolicy)).To(Succeed()) + Expect(nbPolicy.Status.ManagedServiceList).NotTo(ContainElement("default/test-resource")) + + nbPolicy = &netbirdiov1.NBPolicy{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "test-b"}, nbPolicy)).To(Succeed()) + Expect(nbPolicy.Status.ManagedServiceList).NotTo(ContainElement("default/test-resource")) + + nbPolicy = &netbirdiov1.NBPolicy{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "test-c"}, nbPolicy)).To(Succeed()) + Expect(nbPolicy.Status.ManagedServiceList).NotTo(ContainElement("default/test-resource")) + }) + }) + }) + When("Groups are changed", func() { When("Removed groups are no longer referenced by anything", func() { It("should only remove finalizer", func() { diff --git a/internal/util/slices.go b/internal/util/slices.go index 223ed70..19ef3d0 100644 --- a/internal/util/slices.go +++ b/internal/util/slices.go @@ -1,5 +1,7 @@ package util +import "strings" + // Contains return if y is in slice x func Contains[T comparable](x []T, y T) bool { for _, v := range x { @@ -39,3 +41,16 @@ func Equivalent[T comparable](x, y []T) bool { return true } + +// SplitTrim split string and trim whitespace +func SplitTrim(str, sep string) []string { + if len(str) == 0 { + return nil + } + sp := strings.Split(str, sep) + ret := make([]string, 0, len(sp)) + for _, v := range sp { + ret = append(ret, strings.TrimSpace(v)) + } + return ret +}