From c6d0d854dac0379adc879da53cc0a2fb776a784d Mon Sep 17 00:00:00 2001
From: Aleksei Kobzev <kobzonega@yandex.ru>
Date: Fri, 24 Jan 2025 18:57:25 +0800
Subject: [PATCH 1/2] fix bug: missing error handler for arg --auth-token-file

---
 .../unreleased/Added-20250124-141810.yaml     |  3 +
 .../unreleased/Fixed-20250124-141631.yaml     |  3 +
 api/v1alpha1/const.go                         | 14 +++-
 .../controllers/storage/controller_test.go    | 78 +++++++++++++++++++
 internal/resources/database_statefulset.go    | 33 +++-----
 internal/resources/secret.go                  | 22 ------
 internal/resources/storage_statefulset.go     | 33 +++-----
 7 files changed, 119 insertions(+), 67 deletions(-)
 create mode 100644 .changes/unreleased/Added-20250124-141810.yaml
 create mode 100644 .changes/unreleased/Fixed-20250124-141631.yaml

diff --git a/.changes/unreleased/Added-20250124-141810.yaml b/.changes/unreleased/Added-20250124-141810.yaml
new file mode 100644
index 00000000..58e21c15
--- /dev/null
+++ b/.changes/unreleased/Added-20250124-141810.yaml
@@ -0,0 +1,3 @@
+kind: Added
+body: 'annotations overrides default secret name and key for arg --auth-token-file'
+time: 2025-01-24T14:18:10.344319+08:00
diff --git a/.changes/unreleased/Fixed-20250124-141631.yaml b/.changes/unreleased/Fixed-20250124-141631.yaml
new file mode 100644
index 00000000..31c864f9
--- /dev/null
+++ b/.changes/unreleased/Fixed-20250124-141631.yaml
@@ -0,0 +1,3 @@
+kind: Fixed
+body: 'bug: missing error handler for arg --auth-token-file'
+time: 2025-01-24T14:16:31.463111+08:00
diff --git a/api/v1alpha1/const.go b/api/v1alpha1/const.go
index 6f6e9f6a..a9fe280d 100644
--- a/api/v1alpha1/const.go
+++ b/api/v1alpha1/const.go
@@ -28,18 +28,24 @@ const (
 	DiskPathPrefix      = "/dev/kikimr_ssd"
 	DiskNumberMaxDigits = 2
 	DiskFilePath        = "/data"
-	YdbAuthToken        = "ydb-auth-token-file"
 
-	ConfigDir      = "/opt/ydb/cfg"
-	ConfigFileName = "config.yaml"
+	AuthTokenSecretName = "ydb-auth-token-file"
+	AuthTokenSecretKey  = "ydb-auth-token-file"
+	AuthTokenFileArg    = "--auth-token-file"
 
 	DatabaseEncryptionKeySecretDir  = "database_encryption"
 	DatabaseEncryptionKeySecretFile = "key"
 	DatabaseEncryptionKeyConfigFile = "key.txt"
 
+	ConfigDir      = "/opt/ydb/cfg"
+	ConfigFileName = "config.yaml"
+
 	BinariesDir      = "/opt/ydb/bin"
 	DaemonBinaryName = "ydbd"
 
+	AdditionalSecretsDir = "/opt/ydb/secrets"
+	AdditionalVolumesDir = "/opt/ydb/volumes"
+
 	DefaultRootUsername          = "root"
 	DefaultRootPassword          = ""
 	DefaultDatabaseDomain        = "Root"
@@ -60,6 +66,8 @@ const (
 	AnnotationGRPCPublicHost         = "ydb.tech/grpc-public-host"
 	AnnotationNodeHost               = "ydb.tech/node-host"
 	AnnotationNodeDomain             = "ydb.tech/node-domain"
+	AnnotationAuthTokenSecretName    = "ydb.tech/auth-token-secret-name"
+	AnnotationAuthTokenSecretKey     = "ydb.tech/auth-token-secret-key"
 
 	AnnotationValueTrue = "true"
 
diff --git a/internal/controllers/storage/controller_test.go b/internal/controllers/storage/controller_test.go
index e332cc27..5727aad9 100644
--- a/internal/controllers/storage/controller_test.go
+++ b/internal/controllers/storage/controller_test.go
@@ -62,6 +62,23 @@ var _ = Describe("Storage controller medium tests", func() {
 	})
 
 	It("Checking field propagation to objects", func() {
+		getStatefulSet := func(objName string) (appsv1.StatefulSet, error) {
+			foundStatefulSets := appsv1.StatefulSetList{}
+			err := k8sClient.List(ctx, &foundStatefulSets, client.InNamespace(
+				testobjects.YdbNamespace,
+			))
+			if err != nil {
+				return appsv1.StatefulSet{}, err
+			}
+			for _, statefulSet := range foundStatefulSets.Items {
+				if statefulSet.Name == objName {
+					return statefulSet, nil
+				}
+			}
+
+			return appsv1.StatefulSet{}, fmt.Errorf("Statefulset with name %s was not found", objName)
+		}
+
 		storageSample := testobjects.DefaultStorage(filepath.Join("..", "..", "..", "tests", "data", "storage-mirror-3-dc-config.yaml"))
 
 		tmpFilesDir := "/tmp/mounted_volume"
@@ -227,5 +244,66 @@ var _ = Describe("Storage controller medium tests", func() {
 				return len(foundStatefulSets.Items)
 			}, test.Timeout, test.Interval).Should(Equal(1))
 		})
+
+		By("check --auth-token-file arg in StatefulSet...", func() {
+			By("create auth-token Secret with default name...")
+			defaultAuthTokenSecret := &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      v1alpha1.AuthTokenSecretName,
+					Namespace: testobjects.YdbNamespace,
+				},
+				StringData: map[string]string{
+					v1alpha1.AuthTokenSecretKey: "StaffApiUserToken: 'default-token'",
+				},
+			}
+			Expect(k8sClient.Create(ctx, defaultAuthTokenSecret))
+
+			By("append auth-token Secret inside Storage manifest...")
+			Eventually(func() error {
+				foundStorage := v1alpha1.Storage{}
+				Expect(k8sClient.Get(ctx, types.NamespacedName{
+					Name:      testobjects.StorageName,
+					Namespace: testobjects.YdbNamespace,
+				}, &foundStorage))
+				foundStorage.Spec.Secrets = []*corev1.LocalObjectReference{
+					&corev1.LocalObjectReference{
+						Name: v1alpha1.AuthTokenSecretName,
+					},
+				}
+				return k8sClient.Update(ctx, &foundStorage)
+			}, test.Timeout, test.Interval).ShouldNot(HaveOccurred())
+
+			checkAuthTokenArgs := func() error {
+				statefulSet, err := getStatefulSet(testobjects.StorageName)
+				if err != nil {
+					return err
+				}
+				podContainerArgs := statefulSet.Spec.Template.Spec.Containers[0].Args
+				var argExist bool
+				var currentArgValue string
+				authTokenFileArgValue := fmt.Sprintf("%s/%s/%s",
+					v1alpha1.AdditionalSecretsDir,
+					v1alpha1.AuthTokenSecretName,
+					v1alpha1.AuthTokenSecretKey,
+				)
+				for idx, arg := range podContainerArgs {
+					if arg == v1alpha1.AuthTokenFileArg {
+						argExist = true
+						currentArgValue = podContainerArgs[idx+1]
+						break
+					}
+				}
+				if !argExist {
+					return fmt.Errorf("arg `%s` did not found in StatefulSet podTemplate args: %v", v1alpha1.AuthTokenFileArg, podContainerArgs)
+				}
+				if authTokenFileArgValue != currentArgValue {
+					return fmt.Errorf("current arg `%s` value `%s` did not match with expected: %s", v1alpha1.AuthTokenFileArg, currentArgValue, authTokenFileArgValue)
+				}
+				return nil
+			}
+
+			By("check that --auth-token-file arg was added to Statefulset template...")
+			Eventually(checkAuthTokenArgs, test.Timeout, test.Interval).ShouldNot(HaveOccurred())
+		})
 	})
 })
diff --git a/internal/resources/database_statefulset.go b/internal/resources/database_statefulset.go
index 8293a864..06b5cbc5 100644
--- a/internal/resources/database_statefulset.go
+++ b/internal/resources/database_statefulset.go
@@ -1,10 +1,8 @@
 package resources
 
 import (
-	"context"
 	"errors"
 	"fmt"
-	"log"
 	"regexp"
 	"strconv"
 
@@ -619,30 +617,23 @@ func (b *DatabaseStatefulSetBuilder) buildContainerArgs() ([]string, []string) {
 		)
 	}
 
+	authTokenSecretName := api.AuthTokenSecretName
+	authTokenSecretKey := api.AuthTokenSecretKey
+	if value, ok := b.ObjectMeta.Annotations[api.AnnotationAuthTokenSecretName]; ok {
+		authTokenSecretName = value
+	}
+	if value, ok := b.ObjectMeta.Annotations[api.AnnotationAuthTokenSecretKey]; ok {
+		authTokenSecretKey = value
+	}
 	for _, secret := range b.Spec.Secrets {
-		exist, err := CheckSecretKey(
-			context.Background(),
-			b.GetNamespace(),
-			b.RestConfig,
-			&corev1.SecretKeySelector{
-				LocalObjectReference: corev1.LocalObjectReference{
-					Name: secret.Name,
-				},
-				Key: api.YdbAuthToken,
-			},
-		)
-		if err != nil {
-			log.Default().Printf("Failed to inspect a secret %s: %s\n", secret.Name, err.Error())
-			continue
-		}
-		if exist {
+		if secret.Name == authTokenSecretName {
 			args = append(args,
-				"--auth-token-file",
+				api.AuthTokenFileArg,
 				fmt.Sprintf(
 					"%s/%s/%s",
 					wellKnownDirForAdditionalSecrets,
-					secret.Name,
-					api.YdbAuthToken,
+					authTokenSecretName,
+					authTokenSecretKey,
 				),
 			)
 		}
diff --git a/internal/resources/secret.go b/internal/resources/secret.go
index 675f74b8..d9c81a4a 100644
--- a/internal/resources/secret.go
+++ b/internal/resources/secret.go
@@ -13,28 +13,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-func CheckSecretKey(
-	ctx context.Context,
-	namespace string,
-	config *rest.Config,
-	secretKeyRef *corev1.SecretKeySelector,
-) (bool, error) {
-	clientset, err := kubernetes.NewForConfig(config)
-	if err != nil {
-		return false, fmt.Errorf("failed to create kubernetes clientset, error: %w", err)
-	}
-
-	getCtx, cancel := context.WithTimeout(ctx, time.Second)
-	defer cancel()
-	secret, err := clientset.CoreV1().Secrets(namespace).Get(getCtx, secretKeyRef.Name, metav1.GetOptions{})
-	if err != nil {
-		return false, fmt.Errorf("failed to get secret %s, error: %w", secretKeyRef.Name, err)
-	}
-
-	_, exist := secret.Data[secretKeyRef.Key]
-	return exist, nil
-}
-
 func GetSecretKey(
 	ctx context.Context,
 	namespace string,
diff --git a/internal/resources/storage_statefulset.go b/internal/resources/storage_statefulset.go
index 1201d8f7..9c74dcb5 100644
--- a/internal/resources/storage_statefulset.go
+++ b/internal/resources/storage_statefulset.go
@@ -1,10 +1,8 @@
 package resources
 
 import (
-	"context"
 	"errors"
 	"fmt"
-	"log"
 	"strconv"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -521,30 +519,23 @@ func (b *StorageStatefulSetBuilder) buildContainerArgs() ([]string, []string) {
 		)
 	}
 
+	authTokenSecretName := api.AuthTokenSecretName
+	authTokenSecretKey := api.AuthTokenSecretKey
+	if value, ok := b.ObjectMeta.Annotations[api.AnnotationAuthTokenSecretName]; ok {
+		authTokenSecretName = value
+	}
+	if value, ok := b.ObjectMeta.Annotations[api.AnnotationAuthTokenSecretKey]; ok {
+		authTokenSecretKey = value
+	}
 	for _, secret := range b.Spec.Secrets {
-		exist, err := CheckSecretKey(
-			context.Background(),
-			b.GetNamespace(),
-			b.RestConfig,
-			&corev1.SecretKeySelector{
-				LocalObjectReference: corev1.LocalObjectReference{
-					Name: secret.Name,
-				},
-				Key: api.YdbAuthToken,
-			},
-		)
-		if err != nil {
-			log.Default().Printf("Failed to inspect a secret %s: %s\n", secret.Name, err.Error())
-			continue
-		}
-		if exist {
+		if secret.Name == authTokenSecretName {
 			args = append(args,
-				"--auth-token-file",
+				api.AuthTokenFileArg,
 				fmt.Sprintf(
 					"%s/%s/%s",
 					wellKnownDirForAdditionalSecrets,
-					secret.Name,
-					api.YdbAuthToken,
+					authTokenSecretName,
+					authTokenSecretKey,
 				),
 			)
 		}

From 0e514ddb0fda24ad2e073ed9474da255c6c246db Mon Sep 17 00:00:00 2001
From: Aleksei Kobzev <kobzonega@yandex.ru>
Date: Fri, 24 Jan 2025 19:13:13 +0800
Subject: [PATCH 2/2] fix golangci-lint

---
 internal/controllers/storage/controller_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/internal/controllers/storage/controller_test.go b/internal/controllers/storage/controller_test.go
index 5727aad9..b23e2498 100644
--- a/internal/controllers/storage/controller_test.go
+++ b/internal/controllers/storage/controller_test.go
@@ -266,7 +266,7 @@ var _ = Describe("Storage controller medium tests", func() {
 					Namespace: testobjects.YdbNamespace,
 				}, &foundStorage))
 				foundStorage.Spec.Secrets = []*corev1.LocalObjectReference{
-					&corev1.LocalObjectReference{
+					{
 						Name: v1alpha1.AuthTokenSecretName,
 					},
 				}