From 447bbc76cdcc12210787f14b2176fab0320f63cc Mon Sep 17 00:00:00 2001 From: Philip Laine Date: Thu, 25 Jun 2026 10:44:53 +0200 Subject: [PATCH] Ensure netbird sidecar container starts first (#339) This changes the webhook from appending the container to the end to the beginning. Fixes #323 ## Summary by CodeRabbit * **Bug Fixes** * Fixed pod injection ordering: injected components are now placed at the start of their respective lists to ensure correct initialization. * Updated injected sidecar behavior, including setting the restart policy to keep it running consistently. * Adjusted init-container ordering to ensure DNS-related initialization runs first. * **Tests** * Improved coverage by validating both container and sidecar injection modes, including restart policy and injected annotation behavior. --- internal/webhook/v1/pod_webhook.go | 33 +++---- internal/webhook/v1/pod_webhook_test.go | 123 +++++++++++++++--------- 2 files changed, 94 insertions(+), 62 deletions(-) diff --git a/internal/webhook/v1/pod_webhook.go b/internal/webhook/v1/pod_webhook.go index aafe7c1..43751ce 100644 --- a/internal/webhook/v1/pod_webhook.go +++ b/internal/webhook/v1/pod_webhook.go @@ -246,6 +246,21 @@ func (d *PodNetbirdInjector) Default(ctx context.Context, pod *corev1.Pod) error } pod.Spec.Volumes = append(pod.Spec.Volumes, volumes...) + switch sidecarProfile.Spec.InjectionMode { + case nbv1alpha1.InjectionModeSidecar: + container.RestartPolicy = new(corev1.ContainerRestartPolicyAlways) + pod.Spec.InitContainers = slices.Insert(pod.Spec.InitContainers, 0, container) + case nbv1alpha1.InjectionModeContainer: + pod.Spec.Containers = slices.Insert(pod.Spec.Containers, 0, container) + default: + return fmt.Errorf("unknown injection mode %s", sidecarProfile.Spec.InjectionMode) + } + + if pod.Annotations == nil { + pod.Annotations = map[string]string{} + } + pod.Annotations[SidecarProfileAnnotation] = sidecarProfile.Name + resolvInitContainer := corev1.Container{ Name: "resolv-conf", Image: d.clientImage, @@ -263,23 +278,7 @@ func (d *PodNetbirdInjector) Default(ctx context.Context, pod *corev1.Pod) error }, }, } - pod.Spec.InitContainers = append(pod.Spec.InitContainers, resolvInitContainer) - - switch sidecarProfile.Spec.InjectionMode { - case nbv1alpha1.InjectionModeSidecar: - restartPolicy := corev1.ContainerRestartPolicyAlways - container.RestartPolicy = &restartPolicy - pod.Spec.InitContainers = append(pod.Spec.InitContainers, container) - case nbv1alpha1.InjectionModeContainer: - pod.Spec.Containers = append(pod.Spec.Containers, container) - default: - return fmt.Errorf("unknown injection mode %s", sidecarProfile.Spec.InjectionMode) - } - - if pod.Annotations == nil { - pod.Annotations = map[string]string{} - } - pod.Annotations[SidecarProfileAnnotation] = sidecarProfile.Name + pod.Spec.InitContainers = slices.Insert(pod.Spec.InitContainers, 0, resolvInitContainer) return nil } diff --git a/internal/webhook/v1/pod_webhook_test.go b/internal/webhook/v1/pod_webhook_test.go index e9811d4..ca1f5c4 100644 --- a/internal/webhook/v1/pod_webhook_test.go +++ b/internal/webhook/v1/pod_webhook_test.go @@ -21,54 +21,87 @@ import ( func TestPodInjectorSidecarProfile(t *testing.T) { t.Parallel() - setupKey := &nbv1alpha1.SetupKey{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "test", - }, - Spec: nbv1alpha1.SetupKeySpec{ - Name: "test", - Ephemeral: true, - }, - } - sidecarProfile := &nbv1alpha1.SidecarProfile{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "test", - }, - Spec: nbv1alpha1.SidecarProfileSpec{ - SetupKeyRef: corev1.LocalObjectReference{ - Name: "test", - }, - InjectionMode: nbv1alpha1.InjectionModeContainer, - }, - } + for _, mode := range []nbv1alpha1.InjectionMode{nbv1alpha1.InjectionModeContainer, nbv1alpha1.InjectionModeSidecar} { + t.Run(string(mode), func(t *testing.T) { + t.Parallel() - scheme := kruntime.NewScheme() - err := corev1.AddToScheme(scheme) - require.NoError(t, err) - err = nbv1alpha1.AddToScheme(scheme) - require.NoError(t, err) - k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(sidecarProfile, setupKey).Build() - injector := PodNetbirdInjector{ - client: k8sClient, - managementURL: "https://api.netbird.io", - clientImage: "netbirdio/netbird:latest", - } + setupKey := &nbv1alpha1.SetupKey{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + }, + Spec: nbv1alpha1.SetupKeySpec{ + Name: "test", + Ephemeral: true, + }, + } + sidecarProfile := &nbv1alpha1.SidecarProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + }, + Spec: nbv1alpha1.SidecarProfileSpec{ + SetupKeyRef: corev1.LocalObjectReference{ + Name: "test", + }, + InjectionMode: mode, + }, + } - pod := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "test", - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{}, - }, + scheme := kruntime.NewScheme() + err := corev1.AddToScheme(scheme) + require.NoError(t, err) + err = nbv1alpha1.AddToScheme(scheme) + require.NoError(t, err) + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(sidecarProfile, setupKey).Build() + injector := PodNetbirdInjector{ + client: k8sClient, + managementURL: "https://api.netbird.io", + clientImage: "netbirdio/netbird:latest", + } + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + }, + Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + { + Name: "app-init", + }, + }, + Containers: []corev1.Container{ + { + Name: "app", + }, + }, + }, + } + err = injector.Default(t.Context(), pod) + require.NoError(t, err) + + require.EqualT(t, "test", pod.Annotations[SidecarProfileAnnotation]) + switch mode { + case nbv1alpha1.InjectionModeContainer: + require.Len(t, pod.Spec.InitContainers, 2) + require.EqualT(t, "resolv-conf", pod.Spec.InitContainers[0].Name) + require.EqualT(t, "app-init", pod.Spec.InitContainers[1].Name) + require.Len(t, pod.Spec.Containers, 2) + require.EqualT(t, "netbird", pod.Spec.Containers[0].Name) + require.EqualT(t, "app", pod.Spec.Containers[1].Name) + case nbv1alpha1.InjectionModeSidecar: + require.Len(t, pod.Spec.InitContainers, 3) + require.EqualT(t, "resolv-conf", pod.Spec.InitContainers[0].Name) + require.Nil(t, pod.Spec.InitContainers[0].RestartPolicy) + require.EqualT(t, "netbird", pod.Spec.InitContainers[1].Name) + require.EqualT(t, corev1.ContainerRestartPolicyAlways, *pod.Spec.InitContainers[1].RestartPolicy) + require.EqualT(t, "app-init", pod.Spec.InitContainers[2].Name) + require.Len(t, pod.Spec.Containers, 1) + require.EqualT(t, "app", pod.Spec.Containers[0].Name) + } + }) } - err = injector.Default(t.Context(), pod) - require.NoError(t, err) - require.Len(t, pod.Spec.Containers, 1) - require.EqualT(t, "netbird", pod.Spec.Containers[0].Name) } var _ = Describe("Pod Webhook", func() {