From 54b8fc55e15afe0f64896eeb6b5ea83c2d2b313e Mon Sep 17 00:00:00 2001
From: Pedro Juarez <pjuarezd@users.noreply.github.com>
Date: Fri, 23 Feb 2024 15:57:06 -0800
Subject: [PATCH] Detect KES version from image tag to Identify if `v3` should
 be used. (#1993)

A breaking change (https://github.com/minio/kes/pull/414) in KES command arguments is making Operator fail, Operator needs to handle the KES config variations across different KES versions.
---
 api/encryption-handlers.go                    | 60 +++++++++++++++----
 api/tenants_helper_test.go                    | 35 +++++++++--
 pkg/resources/statefulsets/kes-statefulset.go | 11 +++-
 3 files changed, 87 insertions(+), 19 deletions(-)

diff --git a/api/encryption-handlers.go b/api/encryption-handlers.go
index 5d43a9730e3..986e5324adb 100644
--- a/api/encryption-handlers.go
+++ b/api/encryption-handlers.go
@@ -56,15 +56,25 @@ const (
 
 const (
 	// KesConfigVersion1 identifier v1
+	// For KES releases before 2023-04-03T16-41-28Z and versions below v0.20.0
 	KesConfigVersion1 = "v1"
 	// KesConfigVersion2 identifier v2
+	// For KES releases after 2023-04-03T16-41-28Z and versions above v0.20.0
+	// This corresponds to the rename of the `keys` sections to `keystore` in the kes server-config.yaml and some more fields added.
+	// See https://github.com/minio/kes/pull/342
 	KesConfigVersion2 = "v2"
+	// KesConfigVersion3 identifier v3.
+	// For KES releases after 2023-11-09T17-35-47Z
+	// This corresponds to the deprecation of the `--key`, `--cert` and `--auth` kes command arguments.
+	// See https://github.com/minio/kes/pull/414
+	KesConfigVersion3 = "v3"
 )
 
 // KesConfigVersionsMap is a map of kes config version types
 var KesConfigVersionsMap = map[string]interface{}{
 	KesConfigVersion1: kes.ServerConfigV1{},
 	KesConfigVersion2: kes.ServerConfigV2{},
+	KesConfigVersion3: kes.ServerConfigV2{},
 }
 
 type configVersion func(clientCrtIdentity string, encryptionCfg *models.EncryptionConfiguration, mTLSCertificates map[string][]byte) ([]byte, error)
@@ -311,7 +321,7 @@ func tenantEncryptionInfo(ctx context.Context, operatorClient OperatorClientI, c
 				return nil, err
 			}
 			if rawConfiguration, ok := configSecret.Data["server-config.yaml"]; ok {
-				kesConfigVersion, err := getKesConfigVersion(encryptConfig.Image)
+				kesConfigVersion, err := GetKesConfigVersion(encryptConfig.Image)
 				if err != nil {
 					return nil, err
 				}
@@ -683,7 +693,7 @@ func createOrReplaceKesConfigurationSecrets(ctx context.Context, clientSet K8sCl
 		serverRawConfig = []byte(encryptionCfg.Raw)
 		// verify provided configuration is in valid YAML format
 
-		cv, err := getKesConfigVersion(image)
+		cv, err := GetKesConfigVersion(image)
 		if err != nil {
 			return nil, nil, err
 		}
@@ -1105,7 +1115,7 @@ func createKesConfigV2(clientCrtIdentity string, encryptionCfg *models.Encryptio
 
 // getKesConfigMethod identify the config method to use based from the KES image name
 func getKesConfigMethod(image string) (configVersion, error) {
-	version, err := getKesConfigVersion(image)
+	version, err := GetKesConfigVersion(image)
 	if err != nil {
 		return nil, err
 	}
@@ -1118,8 +1128,11 @@ func getKesConfigMethod(image string) (configVersion, error) {
 	}
 }
 
-func getKesConfigVersion(image string) (string, error) {
-	version := KesConfigVersion2
+// GetKesConfigVersion Identifies the "Operator level" KES config version by evaluating the KES container tag.
+// At some point KES versions were published using semantic versioning and date-time versioning later on,
+// this method takes that into consideration to determine the right config to apply to KES containers.
+func GetKesConfigVersion(image string) (string, error) {
+	version := KesConfigVersion3
 
 	imageStrings := strings.Split(image, ":")
 	var imageTag string
@@ -1134,7 +1147,7 @@ func getKesConfigVersion(image string) (string, error) {
 	}
 
 	if imageTag == "latest" {
-		return KesConfigVersion2, nil
+		return KesConfigVersion3, nil
 	}
 
 	// When the image tag is semantic version is config v1
@@ -1143,7 +1156,10 @@ func getKesConfigVersion(image string) (string, error) {
 		if semver.Compare(imageTag, "v0.22.0") < 0 {
 			return KesConfigVersion1, nil
 		}
-		return KesConfigVersion2, nil
+		if semver.Compare(imageTag, "v0.23.0") < 0 {
+			return KesConfigVersion2, nil
+		}
+		return KesConfigVersion3, nil
 	}
 
 	releaseTagNoArch := imageTag
@@ -1157,16 +1173,34 @@ func getKesConfigVersion(image string) (string, error) {
 	}
 
 	// v0.22.0 is the initial image version for Kes config v2, any time format came after and is v2
-	_, err := miniov2.ReleaseTagToReleaseTime(releaseTagNoArch)
+	// v0.23.0 deprecated `--key`, `--cert` and `--auth` flags, for this is v3 config
+	imageVersionTime, err := miniov2.ReleaseTagToReleaseTime(releaseTagNoArch)
 	if err != nil {
 		// could not parse semversion either, returning error
 		return "", fmt.Errorf("could not identify KES version from image TAG: %s", releaseTagNoArch)
 	}
 
-	// Leaving this snippet as comment as this will helpful to compare in future config versions
-	// kesv2ReleaseTime, _ := miniov2.ReleaseTagToReleaseTime("2023-04-03T16-41-28Z")
-	// if imageVersionTime.Before(kesv2ReleaseTime) {
-	// 	version = kesConfigVersion2
-	// }
+	kesv2ReleaseTime, _ := miniov2.ReleaseTagToReleaseTime("2023-04-03T16-41-28Z")
+	kesv3ReleaseTime, _ := miniov2.ReleaseTagToReleaseTime("2023-11-09T17-35-47Z")
+
+	if imageVersionTime.Equal(kesv2ReleaseTime) {
+		return KesConfigVersion2, nil
+	}
+
+	if imageVersionTime.Equal(kesv3ReleaseTime) {
+		return KesConfigVersion3, nil
+	}
+
+	if imageVersionTime.Before(kesv2ReleaseTime) {
+		return KesConfigVersion1, nil
+	}
+
+	if imageVersionTime.Before(kesv3ReleaseTime) {
+		return KesConfigVersion2, nil
+	}
+
+	if imageVersionTime.After(kesv3ReleaseTime) {
+		return KesConfigVersion3, nil
+	}
 	return version, nil
 }
diff --git a/api/tenants_helper_test.go b/api/tenants_helper_test.go
index ca5683e4fd0..e534e3eab7e 100644
--- a/api/tenants_helper_test.go
+++ b/api/tenants_helper_test.go
@@ -388,6 +388,14 @@ func Test_GetConfigVersion(t *testing.T) {
 		wantversion string
 		wantErr     bool
 	}{
+		{
+			name: "error unexpected KES config version",
+			args: args{
+				kesImage: "minio/kes:v0.23.0",
+			},
+			wantversion: KesConfigVersion3,
+			wantErr:     false,
+		},
 		{
 			name: "error unexpected KES config version",
 			args: args{
@@ -409,7 +417,7 @@ func Test_GetConfigVersion(t *testing.T) {
 			args: args{
 				kesImage: "minio/kes:2023-02-15T14-54-37Z",
 			},
-			wantversion: KesConfigVersion2,
+			wantversion: KesConfigVersion1,
 			wantErr:     false,
 		},
 		{
@@ -436,6 +444,23 @@ func Test_GetConfigVersion(t *testing.T) {
 			wantversion: KesConfigVersion2,
 			wantErr:     false,
 		},
+		{
+			name: "error unexpected KES config version",
+			args: args{
+				kesImage: "minio/kes:2023-11-09T17-35-47Z",
+			},
+			wantversion: KesConfigVersion3,
+			wantErr:     false,
+		},
+		{
+			name: "error unexpected KES config version",
+			args: args{
+				kesImage: "minio/kes:2023-11-09T17-35-47Z-arm64",
+			},
+			wantversion: KesConfigVersion3,
+			wantErr:     false,
+		},
+
 		{
 			name: "error unexpected KES config version",
 			args: args{
@@ -449,20 +474,20 @@ func Test_GetConfigVersion(t *testing.T) {
 			args: args{
 				kesImage: "minio/kes:latest",
 			},
-			wantversion: KesConfigVersion2,
+			wantversion: KesConfigVersion3,
 			wantErr:     false,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := getKesConfigVersion(tt.args.kesImage)
+			got, err := GetKesConfigVersion(tt.args.kesImage)
 			if (err != nil) != tt.wantErr {
-				t.Errorf("getKesConfigVersion() error = %v, wantErr %v", err, tt.wantErr)
+				t.Errorf("GetKesConfigVersion() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
 			if !reflect.DeepEqual(got, tt.wantversion) {
-				t.Errorf("getKesConfigVersion() got = %v, want %v", got, tt.wantversion)
+				t.Errorf("GetKesConfigVersion() got = %v, want %v", got, tt.wantversion)
 			}
 		})
 	}
diff --git a/pkg/resources/statefulsets/kes-statefulset.go b/pkg/resources/statefulsets/kes-statefulset.go
index 8eb777e8608..a1adf5b9f90 100644
--- a/pkg/resources/statefulsets/kes-statefulset.go
+++ b/pkg/resources/statefulsets/kes-statefulset.go
@@ -17,6 +17,7 @@ package statefulsets
 import (
 	"sort"
 
+	operatorApi "github.com/minio/operator/api"
 	miniov2 "github.com/minio/operator/pkg/apis/minio.min.io/v2"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -94,7 +95,15 @@ func KESEnvironmentVars(t *miniov2.Tenant) []corev1.EnvVar {
 // KESServerContainer returns the KES container for a KES StatefulSet.
 func KESServerContainer(t *miniov2.Tenant) corev1.Container {
 	// Args to start KES with config mounted at miniov2.KESConfigMountPath and require but don't verify mTLS authentication
-	args := []string{"server", "--config=" + miniov2.KESConfigMountPath + "/server-config.yaml", "--auth=off"}
+	args := []string{"server", "--config=" + miniov2.KESConfigMountPath + "/server-config.yaml"}
+
+	kesVersion, _ := operatorApi.GetKesConfigVersion(t.Spec.KES.Image)
+	// Add `--auth` flag only on config versions that are still compatible with it (v1 and v2).
+	// Starting KES 2023-11-09T17-35-47Z (v3) is no longer supported.
+	switch kesVersion {
+	case operatorApi.KesConfigVersion1, operatorApi.KesConfigVersion2:
+		args = append(args, "--auth=off")
+	}
 
 	return corev1.Container{
 		Name:  miniov2.KESContainerName,