From 6f07d13fdb15d823b1975d129acd07c0cbaa4631 Mon Sep 17 00:00:00 2001 From: Aleksei Kobzev Date: Tue, 28 Jan 2025 01:09:57 +0800 Subject: [PATCH] fix bug: missing error handler for arg --auth-token-file (#283) * fix bug: missing error handler for arg --auth-token-file * fix golangci-lint --- .../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..b23e2498 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{ + { + 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, ), ) }