From cbe0e3a2a1fdddb9ba4385936f9be306aa037748 Mon Sep 17 00:00:00 2001 From: Jan Date: Tue, 3 Mar 2026 08:29:52 +0100 Subject: [PATCH] Add optional privileged mode to NBRoutingPeerSpec (#92) Introduced a new optional boolean field `Privileged` in the `NBRoutingPeerSpec` to allow deployments to specify if containers should run in privileged mode. Updated the CRD, Helm templates, and controller logic to support this feature. A new function `buildSecurityContext` was added to handle the creation of the appropriate security context based on the `Privileged` setting. Tests were updated to cover scenarios where privileged mode is enabled, disabled, or unspecified. This change allows more granular control over container security settings, potentially increasing compatibility with certain workloads that require elevated privileges. see https://github.com/netbirdio/kubernetes-operator/issues/90 **Note:** I am not a Go developer and have no experience with this architecture. I may have overlooked some things. --- api/v1/nbroutingpeer_types.go | 2 + api/v1/zz_generated.deepcopy.go | 5 + .../crds/netbird.io_nbroutingpeers.yaml | 2 + .../templates/nbroutingpeers.yaml | 10 +- helm/netbird-operator-config/values.yaml | 4 +- .../controller/nbroutingpeer_controller.go | 38 +++-- .../nbroutingpeer_controller_test.go | 138 ++++++++++++++++++ 7 files changed, 180 insertions(+), 19 deletions(-) diff --git a/api/v1/nbroutingpeer_types.go b/api/v1/nbroutingpeer_types.go index 89b5eec..8cc8890 100644 --- a/api/v1/nbroutingpeer_types.go +++ b/api/v1/nbroutingpeer_types.go @@ -24,6 +24,8 @@ type NBRoutingPeerSpec struct { Volumes []corev1.Volume `json:"volumes"` // +optional VolumeMounts []corev1.VolumeMount `json:"volumeMounts"` + // +optional + Privileged *bool `json:"privileged,omitempty"` } // NBRoutingPeerStatus defines the observed state of NBRoutingPeer. diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index b36f131..f614b3f 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -539,6 +539,11 @@ func (in *NBRoutingPeerSpec) DeepCopyInto(out *NBRoutingPeerSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Privileged != nil { + in, out := &in.Privileged, &out.Privileged + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NBRoutingPeerSpec. diff --git a/helm/kubernetes-operator/crds/netbird.io_nbroutingpeers.yaml b/helm/kubernetes-operator/crds/netbird.io_nbroutingpeers.yaml index f82e76c..42eae74 100644 --- a/helm/kubernetes-operator/crds/netbird.io_nbroutingpeers.yaml +++ b/helm/kubernetes-operator/crds/netbird.io_nbroutingpeers.yaml @@ -51,6 +51,8 @@ spec: additionalProperties: type: string type: object + privileged: + type: boolean replicas: format: int32 type: integer diff --git a/helm/netbird-operator-config/templates/nbroutingpeers.yaml b/helm/netbird-operator-config/templates/nbroutingpeers.yaml index 940938f..bb0ab93 100644 --- a/helm/netbird-operator-config/templates/nbroutingpeers.yaml +++ b/helm/netbird-operator-config/templates/nbroutingpeers.yaml @@ -13,7 +13,7 @@ metadata: name: router namespace: {{ $k }} {{ $spec := merge $defaults $v }} -{{- if or (or (or $spec.replicas $spec.resources) (or $spec.labels $spec.annotations)) (or $spec.nodeSelector $spec.tolerations) }} +{{- if or (or (or $spec.replicas $spec.resources) (or $spec.labels $spec.annotations)) (or (or $spec.nodeSelector $spec.tolerations) $spec.privileged) }} spec: {{- if $spec.replicas }} replicas: {{ $spec.replicas }} @@ -38,6 +38,9 @@ spec: tolerations: {{- toYaml $spec.tolerations | nindent 4 }} {{- end }} + {{- if $spec.privileged }} + privileged: {{ $spec.privileged }} + {{- end }} {{- end }} --- {{- end }} @@ -52,7 +55,7 @@ metadata: app.kubernetes.io/component: operator {{- include "netbird-operator-config.labels" $ | nindent 4 }} name: router -{{- if or (or (or .replicas .resources) (or .labels .annotations)) (or .nodeSelector .tolerations) }} +{{- if or (or (or .replicas .resources) (or .labels .annotations)) (or (or .nodeSelector .tolerations) .privileged) }} spec: {{- if .replicas }} replicas: {{ .replicas }} @@ -77,6 +80,9 @@ spec: tolerations: {{- toYaml .tolerations | nindent 4 }} {{- end }} + {{- if .privileged }} + privileged: {{ .privileged }} + {{- end }} {{- end }} {{- end }} {{- end }} diff --git a/helm/netbird-operator-config/values.yaml b/helm/netbird-operator-config/values.yaml index b9c187a..7ed049a 100644 --- a/helm/netbird-operator-config/values.yaml +++ b/helm/netbird-operator-config/values.yaml @@ -20,6 +20,7 @@ router: # annotations: {} # nodeSelector: {} # tolerations: [] + # privileged: false # Only needed if namespacedNetworks is set to true namespaces: {} # default: @@ -34,7 +35,8 @@ router: # labels: {} # annotations: {} # nodeSelector: {} - # tolerations: [] + # tolerations: [] + # privileged: false # NetBird Policies for use with exposed services policies: {} # default: diff --git a/internal/controller/nbroutingpeer_controller.go b/internal/controller/nbroutingpeer_controller.go index 8277899..2c36962 100644 --- a/internal/controller/nbroutingpeer_controller.go +++ b/internal/controller/nbroutingpeer_controller.go @@ -187,15 +187,9 @@ func (r *NBRoutingPeerReconciler) handleDeployment(ctx context.Context, req ctrl Value: r.ManagementURL, }, }, - SecurityContext: &corev1.SecurityContext{ - Capabilities: &corev1.Capabilities{ - Add: []corev1.Capability{ - "NET_ADMIN", - }, - }, - }, - Resources: nbrp.Spec.Resources, - VolumeMounts: nbrp.Spec.VolumeMounts, + SecurityContext: r.buildSecurityContext(nbrp), + Resources: nbrp.Spec.Resources, + VolumeMounts: nbrp.Spec.VolumeMounts, }, }, Volumes: nbrp.Spec.Volumes, @@ -263,13 +257,7 @@ func (r *NBRoutingPeerReconciler) handleDeployment(ctx context.Context, req ctrl Value: r.ManagementURL, }, } - updatedDeployment.Spec.Template.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{ - Capabilities: &corev1.Capabilities{ - Add: []corev1.Capability{ - "NET_ADMIN", - }, - }, - } + updatedDeployment.Spec.Template.Spec.Containers[0].SecurityContext = r.buildSecurityContext(nbrp) updatedDeployment.Spec.Template.Spec.Containers[0].Resources = nbrp.Spec.Resources updatedDeployment.Spec.Template.Spec.Containers[0].VolumeMounts = nbrp.Spec.VolumeMounts @@ -657,6 +645,24 @@ func (r *NBRoutingPeerReconciler) handleDelete(ctx context.Context, req ctrl.Req return ctrl.Result{}, nil } +// buildSecurityContext creates the appropriate SecurityContext based on the NBRoutingPeer spec +func (r *NBRoutingPeerReconciler) buildSecurityContext(nbrp *netbirdiov1.NBRoutingPeer) *corev1.SecurityContext { + securityContext := &corev1.SecurityContext{ + Capabilities: &corev1.Capabilities{ + Add: []corev1.Capability{ + "NET_ADMIN", + }, + }, + } + + // Set privileged mode if specified + if nbrp.Spec.Privileged != nil && *nbrp.Spec.Privileged { + securityContext.Privileged = nbrp.Spec.Privileged + } + + return securityContext +} + // SetupWithManager sets up the controller with the Manager. func (r *NBRoutingPeerReconciler) SetupWithManager(mgr ctrl.Manager) error { r.netbird = netbird.New(r.ManagementURL, r.APIKey) diff --git a/internal/controller/nbroutingpeer_controller_test.go b/internal/controller/nbroutingpeer_controller_test.go index dbb7428..04764e8 100644 --- a/internal/controller/nbroutingpeer_controller_test.go +++ b/internal/controller/nbroutingpeer_controller_test.go @@ -915,6 +915,144 @@ var _ = Describe("NBRoutingPeer Controller", func() { Expect(deployment.ResourceVersion).To(Equal(resourceVersion)) }) }) + + When("Privileged mode is enabled", func() { + It("should create deployment with privileged security context", func() { + Expect(k8sClient.Get(ctx, typeNamespacedName, nbroutingpeer)).To(Succeed()) + nbroutingpeer.Spec.Privileged = util.Ptr(true) + Expect(k8sClient.Update(ctx, nbroutingpeer)).To(Succeed()) + + _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + deployment := &appsv1.Deployment{} + Expect(k8sClient.Get(ctx, typeNamespacedName, deployment)).To(Succeed()) + Expect(deployment.Spec.Template.Spec.Containers).To(HaveLen(1)) + + container := deployment.Spec.Template.Spec.Containers[0] + Expect(container.SecurityContext).NotTo(BeNil()) + Expect(container.SecurityContext.Privileged).NotTo(BeNil()) + Expect(*container.SecurityContext.Privileged).To(BeTrue()) + Expect(container.SecurityContext.Capabilities).NotTo(BeNil()) + Expect(container.SecurityContext.Capabilities.Add).To(ContainElement(corev1.Capability("NET_ADMIN"))) + }) + }) + + When("Privileged mode is disabled", func() { + It("should create deployment with non-privileged security context", func() { + Expect(k8sClient.Get(ctx, typeNamespacedName, nbroutingpeer)).To(Succeed()) + nbroutingpeer.Spec.Privileged = util.Ptr(false) + Expect(k8sClient.Update(ctx, nbroutingpeer)).To(Succeed()) + + _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + deployment := &appsv1.Deployment{} + Expect(k8sClient.Get(ctx, typeNamespacedName, deployment)).To(Succeed()) + Expect(deployment.Spec.Template.Spec.Containers).To(HaveLen(1)) + + container := deployment.Spec.Template.Spec.Containers[0] + Expect(container.SecurityContext).NotTo(BeNil()) + Expect(container.SecurityContext.Privileged).To(BeNil()) + Expect(container.SecurityContext.Capabilities).NotTo(BeNil()) + Expect(container.SecurityContext.Capabilities.Add).To(ContainElement(corev1.Capability("NET_ADMIN"))) + }) + }) + + When("Privileged mode is not specified", func() { + It("should create deployment with default security context (non-privileged)", func() { + // Ensure Privileged is nil (default) + Expect(k8sClient.Get(ctx, typeNamespacedName, nbroutingpeer)).To(Succeed()) + nbroutingpeer.Spec.Privileged = nil + Expect(k8sClient.Update(ctx, nbroutingpeer)).To(Succeed()) + + _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + deployment := &appsv1.Deployment{} + Expect(k8sClient.Get(ctx, typeNamespacedName, deployment)).To(Succeed()) + Expect(deployment.Spec.Template.Spec.Containers).To(HaveLen(1)) + + container := deployment.Spec.Template.Spec.Containers[0] + Expect(container.SecurityContext).NotTo(BeNil()) + Expect(container.SecurityContext.Privileged).To(BeNil()) + Expect(container.SecurityContext.Capabilities).NotTo(BeNil()) + Expect(container.SecurityContext.Capabilities.Add).To(ContainElement(corev1.Capability("NET_ADMIN"))) + }) + }) + + When("Deployment exists and privileged mode changes", func() { + It("should update deployment security context when privileged mode is enabled", func() { + // First create deployment without privileged mode + _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + // Verify initial state + deployment := &appsv1.Deployment{} + Expect(k8sClient.Get(ctx, typeNamespacedName, deployment)).To(Succeed()) + container := deployment.Spec.Template.Spec.Containers[0] + Expect(container.SecurityContext.Privileged).To(BeNil()) + + // Enable privileged mode + Expect(k8sClient.Get(ctx, typeNamespacedName, nbroutingpeer)).To(Succeed()) + nbroutingpeer.Spec.Privileged = util.Ptr(true) + Expect(k8sClient.Update(ctx, nbroutingpeer)).To(Succeed()) + + _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + // Verify privileged mode is now enabled + deployment = &appsv1.Deployment{} + Expect(k8sClient.Get(ctx, typeNamespacedName, deployment)).To(Succeed()) + container = deployment.Spec.Template.Spec.Containers[0] + Expect(container.SecurityContext.Privileged).NotTo(BeNil()) + Expect(*container.SecurityContext.Privileged).To(BeTrue()) + }) + + It("should update deployment security context when privileged mode is disabled", func() { + // First create deployment with privileged mode enabled + nbroutingpeer.Spec.Privileged = util.Ptr(true) + Expect(k8sClient.Update(ctx, nbroutingpeer)).To(Succeed()) + + _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + // Verify initial privileged state + deployment := &appsv1.Deployment{} + Expect(k8sClient.Get(ctx, typeNamespacedName, deployment)).To(Succeed()) + container := deployment.Spec.Template.Spec.Containers[0] + Expect(container.SecurityContext.Privileged).NotTo(BeNil()) + Expect(*container.SecurityContext.Privileged).To(BeTrue()) + + // Disable privileged mode + Expect(k8sClient.Get(ctx, typeNamespacedName, nbroutingpeer)).To(Succeed()) + nbroutingpeer.Spec.Privileged = util.Ptr(false) + Expect(k8sClient.Update(ctx, nbroutingpeer)).To(Succeed()) + + _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + // Verify privileged mode is now disabled + deployment = &appsv1.Deployment{} + Expect(k8sClient.Get(ctx, typeNamespacedName, deployment)).To(Succeed()) + container = deployment.Spec.Template.Spec.Containers[0] + Expect(container.SecurityContext.Privileged).To(BeNil()) + }) + }) }) When("NBRoutingPeer is set for deletion", func() { networkDeleted := false