From 3fcc4fa475fc486cac4a58a1fb54950eb2636943 Mon Sep 17 00:00:00 2001
From: say5 <alexandr.swiridov@gmail.com>
Date: Wed, 18 Dec 2024 15:02:56 +0100
Subject: [PATCH] chore: refactor to use IsNativeSidecarSupport function

---
 pkg/pod/pod.go                    | 23 ++++++----
 pkg/pod/pod_test.go               | 72 +++++++++++++++++++++++++++++++
 pkg/reconciler/taskrun/taskrun.go | 10 +----
 3 files changed, 88 insertions(+), 17 deletions(-)

diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go
index 1c7123f7156..002f6d08ad7 100644
--- a/pkg/pod/pod.go
+++ b/pkg/pod/pod.go
@@ -37,6 +37,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/version"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/utils/strings/slices"
 	"knative.dev/pkg/changeset"
@@ -433,10 +434,6 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1.TaskRun, taskSpec v1.Ta
 	mergedPodContainers := stepContainers
 	mergedPodInitContainers := initContainers
 
-	// Check if current k8s version is less than 1.29
-	// Since Kubernetes Major version cannot be 0 and if it's 2 then sidecar will be in
-	// we are only concerned about major version 1 and if the minor is less than 29 then
-	// we need to do the current logic
 	useTektonSidecar := true
 	if config.FromContextOrDefaults(ctx).FeatureFlags.EnableKubernetesSidecar {
 		// Go through the logic for enable-kubernetes feature flag
@@ -446,10 +443,7 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1.TaskRun, taskSpec v1.Ta
 		if err != nil {
 			return nil, err
 		}
-		svMinor := strings.TrimSuffix(sv.Minor, "+") // Remove '+' if present
-		svMajorInt, _ := strconv.Atoi(sv.Major)
-		svMinorInt, _ := strconv.Atoi(svMinor)
-		if svMajorInt >= 1 && svMinorInt >= SidecarK8sMinorVersionCheck {
+		if IsNativeSidecarSupport(sv) {
 			// Add RestartPolicy and Merge into initContainer
 			useTektonSidecar = false
 			for i := range sidecarContainers {
@@ -736,3 +730,16 @@ func artifactPathReferencedInStep(step v1.Step) bool {
 	}
 	return false
 }
+
+// isNativeSidecarSupport returns true if k8s api has native sidecar support
+// based on the k8s version (1.29+).
+// See https://kubernetes.io/docs/concepts/workloads/pods/sidecar-containers/ for more info.
+func IsNativeSidecarSupport(serverVersion *version.Info) bool {
+	minor := strings.TrimSuffix(serverVersion.Minor, "+") // Remove '+' if present
+	majorInt, _ := strconv.Atoi(serverVersion.Major)
+	minorInt, _ := strconv.Atoi(minor)
+	if (majorInt == 1 && minorInt >= SidecarK8sMinorVersionCheck) || majorInt > 1 {
+		return true
+	}
+	return false
+}
diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go
index 4b5fc667cc0..373d8ab1f70 100644
--- a/pkg/pod/pod_test.go
+++ b/pkg/pod/pod_test.go
@@ -3767,3 +3767,75 @@ func TestPodBuildWithK8s129(t *testing.T) {
 		t.Errorf("Sidecar does not have RestartPolicy Always: %s", diff.PrintWantGot(d))
 	}
 }
+func TestIsNativeSidecarSupport(t *testing.T) {
+	tests := []struct {
+		name          string
+		serverVersion *version.Info
+		want          bool
+	}{
+		{
+			name: "Kubernetes version 1.29",
+			serverVersion: &version.Info{
+				Major: "1",
+				Minor: "29",
+			},
+			want: true,
+		},
+		{
+			name: "Kubernetes version 1.30",
+			serverVersion: &version.Info{
+				Major: "1",
+				Minor: "30",
+			},
+			want: true,
+		},
+		{
+			name: "Kubernetes version 2.0",
+			serverVersion: &version.Info{
+				Major: "2",
+				Minor: "0",
+			},
+			want: true,
+		},
+		{
+			name: "Kubernetes version 1.28",
+			serverVersion: &version.Info{
+				Major: "1",
+				Minor: "28",
+			},
+			want: false,
+		},
+		{
+			name: "Kubernetes version 1.29+",
+			serverVersion: &version.Info{
+				Major: "1",
+				Minor: "29+",
+			},
+			want: true,
+		},
+		{
+			name: "Kubernetes version 1.28+",
+			serverVersion: &version.Info{
+				Major: "1",
+				Minor: "28+",
+			},
+			want: false,
+		},
+		{
+			name: "Kubernetes version 0.29",
+			serverVersion: &version.Info{
+				Major: "0",
+				Minor: "29",
+			},
+			want: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := IsNativeSidecarSupport(tt.serverVersion); got != tt.want {
+				t.Errorf("IsNativeSidecarSupport() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go
index f0d7683e880..f977315be16 100644
--- a/pkg/reconciler/taskrun/taskrun.go
+++ b/pkg/reconciler/taskrun/taskrun.go
@@ -22,7 +22,6 @@ import (
 	"fmt"
 	"reflect"
 	"slices"
-	"strconv"
 	"strings"
 	"time"
 
@@ -153,10 +152,6 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1.TaskRun) pkgrecon
 		// and may not have had all of the assumed default specified.
 		tr.SetDefaults(ctx)
 
-		// Check if current k8s version is less than 1.29
-		// Since Kubernetes Major version cannot be 0 and if it's 2 then sidecar will be in
-		// we are only concerned about major version 1 and if the minor is less than 29 then
-		// we need to do the current logic
 		useTektonSidecar := true
 		if config.FromContextOrDefaults(ctx).FeatureFlags.EnableKubernetesSidecar {
 			dc := c.KubeClientSet.Discovery()
@@ -164,10 +159,7 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1.TaskRun) pkgrecon
 			if err != nil {
 				return err
 			}
-			svMinor := strings.TrimSuffix(sv.Minor, "+") // Remove '+' if present
-			svMajorInt, _ := strconv.Atoi(sv.Major)
-			svMinorInt, _ := strconv.Atoi(svMinor)
-			if svMajorInt >= 1 && svMinorInt >= podconvert.SidecarK8sMinorVersionCheck {
+			if podconvert.IsNativeSidecarSupport(sv) {
 				useTektonSidecar = false
 				logger.Infof("Using Kubernetes Native Sidecars \n")
 			}