feat: allow proxies to be injected using a native sidecar

Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>

Author: devjoes

cmd/webhook/main.go

@@ -168,7 +168,7 @@ func setupWebhook(mgr manager.Manager, setupFinished chan struct{}) {
 
 	// setup webhooks
 	entryLog.Info("registering webhook to the webhook server")
-	podMutator, err := wh.NewPodMutator(mgr.GetClient(), mgr.GetAPIReader(), audience, mgr.GetScheme())
+	podMutator, err := wh.NewPodMutator(mgr.GetClient(), mgr.GetAPIReader(), audience, mgr.GetScheme(), mgr.GetConfig())
 	if err != nil {
 		panic(fmt.Errorf("unable to set up pod mutator: %w", err))
 	}

pkg/webhook/webhook.go

@@ -20,6 +20,8 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/discovery"
+	"k8s.io/client-go/rest"
 	"k8s.io/utils/ptr"
 
 	"github.com/Azure/azure-workload-identity/pkg/config"
@@ -53,17 +55,31 @@ type podMutator struct {
 	azureAuthorityHost string
 	proxyImage         string
 	proxyInitImage     string
+	useNativeSidecar   bool
 }
 
 // NewPodMutator returns a pod mutation handler
-func NewPodMutator(client client.Client, reader client.Reader, audience string, scheme *runtime.Scheme) (admission.Handler, error) {
+func NewPodMutator(client client.Client, reader client.Reader, audience string, scheme *runtime.Scheme, restConfig *rest.Config) (admission.Handler, error) {
 	c, err := config.ParseConfig()
 	if err != nil {
 		return nil, err
 	}
 	if audience == "" {
 		audience = DefaultAudience
 	}
+
+	discoveryClient, err := discovery.NewDiscoveryClientForConfig(restConfig)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to create discovery client")
+	}
+	// "SidecarContainers" went beta in 1.29. With the 3 version skew policy,
+	// between API server and kubelet, 1.32 is the earliest version this can be
+	// safely used.
+	useNativeSidecar, err := isSupportedKubernetesVersion(discoveryClient, 1, 32)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to check kubernetes version")
+	}
+
 	// this is used to configure the AZURE_AUTHORITY_HOST env var that's
 	// used by the azure sdk
 	azureAuthorityHost, err := getAzureAuthorityHost(c)
@@ -92,6 +108,7 @@ func NewPodMutator(client client.Client, reader client.Reader, audience string,
 		azureAuthorityHost: azureAuthorityHost,
 		proxyImage:         proxyImage,
 		proxyInitImage:     proxyInitImage,
+		useNativeSidecar:   useNativeSidecar,
 	}, nil
 }
 
@@ -155,7 +172,11 @@ func (m *podMutator) Handle(ctx context.Context, req admission.Request) (respons
 		}
 
 		pod.Spec.InitContainers = m.injectProxyInitContainer(pod.Spec.InitContainers, proxyPort)
-		pod.Spec.Containers = m.injectProxySidecarContainer(pod.Spec.Containers, proxyPort)
+		if m.useNativeSidecar {
+			pod.Spec.InitContainers = m.injectProxySidecarContainer(pod.Spec.InitContainers, proxyPort, ptr.To(corev1.ContainerRestartPolicyAlways))
+		} else {
+			pod.Spec.Containers = m.injectProxySidecarContainer(pod.Spec.Containers, proxyPort, nil)
+		}
 	}
 
 	// get service account token expiration
@@ -228,13 +249,14 @@ func (m *podMutator) injectProxyInitContainer(containers []corev1.Container, pro
 	return containers
 }
 
-func (m *podMutator) injectProxySidecarContainer(containers []corev1.Container, proxyPort int32) []corev1.Container {
+func (m *podMutator) injectProxySidecarContainer(containers []corev1.Container, proxyPort int32, restartPolicy *corev1.ContainerRestartPolicy) []corev1.Container {
 	for _, container := range containers {
 		if container.Name == ProxySidecarContainerName {
 			return containers
 		}
 	}
 	logLevel := currentLogLevel() // run the proxy at the same log level as the webhook
+
 	containers = append([]corev1.Container{{
 		Name:            ProxySidecarContainerName,
 		Image:           m.proxyImage,
@@ -267,6 +289,7 @@ func (m *podMutator) injectProxySidecarContainer(containers []corev1.Container,
 			ReadOnlyRootFilesystem: ptr.To(true),
 			RunAsNonRoot:           ptr.To(true),
 		},
+		RestartPolicy: restartPolicy,
 	}}, containers...)
 
 	return containers
@@ -467,3 +490,26 @@ func currentLogLevel() string {
 	}
 	return "" // this is unreachable
 }
+
+// isSupportedKubernetesVersion checks if the Kubernetes version is supported
+// by comparing the major and minor version numbers.
+// It returns true if the version is greater than or equal to the specified
+// minimum version, and false otherwise.
+func isSupportedKubernetesVersion(discoveryClient discovery.DiscoveryInterface, minMajor, minMinor int) (bool, error) {
+	// check if the kubernetes version is supported
+	version, err := discoveryClient.ServerVersion()
+	if err != nil {
+		return false, err
+	}
+
+	major, err := strconv.Atoi(version.Major)
+	if err != nil {
+		return false, err
+	}
+	minor, err := strconv.Atoi(version.Minor)
+	if err != nil {
+		return false, err
+	}
+
+	return major > minMajor || (major == minMajor && minor >= minMinor), nil
+}

pkg/webhook/webhook_test.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 	"testing"
 
+	"k8s.io/utils/ptr"
 	"monis.app/mlog"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -19,7 +20,9 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/utils/ptr"
+	"k8s.io/apimachinery/pkg/version"
+	discoveryfake "k8s.io/client-go/discovery/fake"
+	kubernetesfake "k8s.io/client-go/kubernetes/fake"
 
 	"github.com/Azure/azure-workload-identity/pkg/config"
 )
@@ -1115,20 +1118,26 @@ func TestInjectProxySidecarContainer(t *testing.T) {
 		},
 	}
 
+	proxyNativeSidecarContainer := proxySidecarContainer
+	proxyNativeSidecarContainer.RestartPolicy = ptr.To(corev1.ContainerRestartPolicyAlways)
+
 	tests := []struct {
 		name               string
 		containers         []corev1.Container
 		expectedContainers []corev1.Container
+		restartPolicy      *corev1.ContainerRestartPolicy
 	}{
 		{
 			name:               "no containers",
 			containers:         []corev1.Container{},
 			expectedContainers: []corev1.Container{proxySidecarContainer},
+			restartPolicy:      nil,
 		},
 		{
 			name:               "proxy sidecar container manually injected",
 			containers:         []corev1.Container{proxySidecarContainer},
 			expectedContainers: []corev1.Container{proxySidecarContainer},
+			restartPolicy:      nil,
 		},
 		{
 			name: "inject proxy sidecar container to existing containers",
@@ -1145,13 +1154,31 @@ func TestInjectProxySidecarContainer(t *testing.T) {
 					Image: "my-image",
 				},
 			},
+			restartPolicy: nil,
+		},
+		{
+			name: "inject proxy native sidecar container to existing containers when restartPolicy is set",
+			containers: []corev1.Container{
+				{
+					Name:  "my-container",
+					Image: "my-image",
+				},
+			},
+			expectedContainers: []corev1.Container{
+				proxyNativeSidecarContainer,
+				{
+					Name:  "my-container",
+					Image: "my-image",
+				},
+			},
+			restartPolicy: ptr.To(corev1.ContainerRestartPolicyAlways),
 		},
 	}
 
 	m := &podMutator{proxyImage: imageURL}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			containers := m.injectProxySidecarContainer(test.containers, proxyPort)
+			containers := m.injectProxySidecarContainer(test.containers, proxyPort, test.restartPolicy)
 			if !reflect.DeepEqual(containers, test.expectedContainers) {
 				t.Errorf("expected: %v, got: %v", test.expectedContainers, containers)
 			}
@@ -1374,3 +1401,81 @@ func TestHandleError(t *testing.T) {
 		})
 	}
 }
+
+func TestIsSupportedKubernetesVersion(t *testing.T) {
+	tests := []struct {
+		name        string
+		versionInfo *version.Info
+		minMajor    int
+		minMinor    int
+		want        bool
+		wantErr     bool
+	}{
+		{
+			name:        "Exact match",
+			versionInfo: &version.Info{Major: "1", Minor: "20"},
+			minMajor:    1,
+			minMinor:    20,
+			want:        true,
+		},
+		{
+			name:        "Higher major version",
+			versionInfo: &version.Info{Major: "2", Minor: "0"},
+			minMajor:    1,
+			minMinor:    25,
+			want:        true,
+		},
+		{
+			name:        "Higher minor version",
+			versionInfo: &version.Info{Major: "1", Minor: "25"},
+			minMajor:    1,
+			minMinor:    20,
+			want:        true,
+		},
+		{
+			name:        "Lower minor version",
+			versionInfo: &version.Info{Major: "1", Minor: "18"},
+			minMajor:    1,
+			minMinor:    20,
+			want:        false,
+		},
+		{
+			name:        "Lower major version",
+			versionInfo: &version.Info{Major: "0", Minor: "25"},
+			minMajor:    1,
+			minMinor:    20,
+			want:        false,
+		},
+		{
+			name:        "Invalid major version",
+			versionInfo: &version.Info{Major: "invalid", Minor: "20"},
+			minMajor:    1,
+			minMinor:    20,
+			wantErr:     true,
+		},
+		{
+			name:        "Invalid minor version",
+			versionInfo: &version.Info{Major: "1", Minor: "invalid"},
+			minMajor:    1,
+			minMinor:    20,
+			wantErr:     true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			discoveryClient := kubernetesfake.NewClientset().Discovery()
+			discoveryClient.(*discoveryfake.FakeDiscovery).FakedServerVersion = tt.versionInfo
+
+			got, err := isSupportedKubernetesVersion(discoveryClient, tt.minMajor, tt.minMinor)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("isSupportedKubernetesVersion() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if got != tt.want {
+				t.Errorf("isSupportedKubernetesVersion() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}