Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix bug: missing error handler for arg --auth-token-file #283

Merged
merged 2 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changes/unreleased/Added-20250124-141810.yaml
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions .changes/unreleased/Fixed-20250124-141631.yaml
Original file line number Diff line number Diff line change
@@ -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
14 changes: 11 additions & 3 deletions api/v1alpha1/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two vars (and maybe more) seems to be defined as wellKnownDirForAdditionalSecrets as well:

wellKnownDirForAdditionalSecrets = "/opt/ydb/secrets"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It remains here for compatibility. One of the next changes will be switch to use this const from package api/v1alpha

AdditionalVolumesDir = "/opt/ydb/volumes"

DefaultRootUsername = "root"
DefaultRootPassword = ""
DefaultDatabaseDomain = "Root"
Expand All @@ -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"

Expand Down
78 changes: 78 additions & 0 deletions internal/controllers/storage/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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())
})
})
})
33 changes: 12 additions & 21 deletions internal/resources/database_statefulset.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
package resources

import (
"context"
"errors"
"fmt"
"log"
"regexp"
"strconv"

Expand Down Expand Up @@ -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,
),
)
}
Expand Down
22 changes: 0 additions & 22 deletions internal/resources/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
33 changes: 12 additions & 21 deletions internal/resources/storage_statefulset.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
package resources

import (
"context"
"errors"
"fmt"
"log"
"strconv"

appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -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,
),
)
}
Expand Down
Loading