Просмотр исходного кода

fix(volcengine): honor secretRef.namespace for ClusterSecretStore (#6423)

The Volcengine provider resolved auth credentials using only the
namespace passed by the controller, ignoring secretRef.namespace on the
SecretKeySelector. For a ClusterSecretStore the controller passes an
empty namespace, so credential lookups failed with:

    failed to get secret <name> in namespace :
    an empty namespace may not be set when a resource name is provided

getSecretValue now takes the store kind and, for a ClusterSecretStore,
uses secretRef.namespace when set. A namespaced SecretStore still ignores
it, preserving namespace isolation.

Fixes #6310

Signed-off-by: Ruslan M <ruslan@lightningstep.com>
Co-authored-by: Ruslan M <ruslan@lightningstep.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Gergely Bräutigam <gergely.brautigam@sap.com>
Ruslan Maiboroda 1 неделя назад
Родитель
Сommit
6c144f4bcf

+ 7 - 29
providers/v1/volcengine/auth.go

@@ -25,19 +25,17 @@ import (
 	"github.com/volcengine/volcengine-go-sdk/volcengine"
 	"github.com/volcengine/volcengine-go-sdk/volcengine/credentials"
 	"github.com/volcengine/volcengine-go-sdk/volcengine/session"
-	v1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
-	esmeta "github.com/external-secrets/external-secrets/apis/meta/v1"
+	"github.com/external-secrets/external-secrets/runtime/esutils/resolvers"
 )
 
 // NewSession creates a new Volcengine session based on the provider configuration.
 // It follows the credential chain:
 // 1. Static credentials from a Kubernetes secret (if specified in auth.secretRef).
 // 2. IRSA (IAM Role for Service Account) via environment variables (if auth.secretRef is not specified).
-func NewSession(ctx context.Context, provider *esv1.VolcengineProvider, kube client.Client, namespace string) (*session.Session, error) {
+func NewSession(ctx context.Context, provider *esv1.VolcengineProvider, kube client.Client, storeKind, namespace string) (*session.Session, error) {
 	if provider == nil {
 		return nil, errors.New("volcengine provider can not be nil")
 	}
@@ -49,22 +47,22 @@ func NewSession(ctx context.Context, provider *esv1.VolcengineProvider, kube cli
 
 	if provider.Auth != nil && provider.Auth.SecretRef != nil {
 		// If SecretRef is provided, use static credentials.
-		accessKeyID, err := getSecretValue(ctx, kube, namespace, provider.Auth.SecretRef.AccessKeyID)
+		accessKeyID, err := resolvers.SecretKeyRef(ctx, kube, storeKind, namespace, &provider.Auth.SecretRef.AccessKeyID)
 		if err != nil {
 			return nil, fmt.Errorf("failed to get accessKeyID: %w", err)
 		}
-		secretAccessKey, err := getSecretValue(ctx, kube, namespace, provider.Auth.SecretRef.SecretAccessKey)
+		secretAccessKey, err := resolvers.SecretKeyRef(ctx, kube, storeKind, namespace, &provider.Auth.SecretRef.SecretAccessKey)
 		if err != nil {
 			return nil, fmt.Errorf("failed to get secretAccessKey: %w", err)
 		}
-		token := []byte{}
+		token := ""
 		if provider.Auth.SecretRef.Token != nil {
-			token, err = getSecretValue(ctx, kube, namespace, *provider.Auth.SecretRef.Token)
+			token, err = resolvers.SecretKeyRef(ctx, kube, storeKind, namespace, provider.Auth.SecretRef.Token)
 			if err != nil {
 				return nil, fmt.Errorf("failed to get token: %w", err)
 			}
 		}
-		creds = credentials.NewStaticCredentials(string(accessKeyID), string(secretAccessKey), string(token))
+		creds = credentials.NewStaticCredentials(accessKeyID, secretAccessKey, token)
 	} else {
 		// If SecretRef is not provided, automatically use the default credential chain,
 		// which includes environment variables and IRSA.
@@ -80,23 +78,3 @@ func NewSession(ctx context.Context, provider *esv1.VolcengineProvider, kube cli
 	}
 	return sess, nil
 }
-
-// getSecretValue retrieves a value from a Kubernetes secret.
-func getSecretValue(ctx context.Context, kube client.Client, namespace string, secretSelector esmeta.SecretKeySelector) ([]byte, error) {
-	secret := &v1.Secret{}
-	ref := types.NamespacedName{
-		Namespace: namespace,
-		Name:      secretSelector.Name,
-	}
-
-	if err := kube.Get(ctx, ref, secret); err != nil {
-		return nil, fmt.Errorf("failed to get secret %s in namespace %s: %w", ref.Name, ref.Namespace, err)
-	}
-
-	value, ok := secret.Data[secretSelector.Key]
-	if !ok {
-		return nil, fmt.Errorf("key %q not found in secret %s in namespace %s", secretSelector.Key, ref.Name, ref.Namespace)
-	}
-
-	return value, nil
-}

+ 98 - 12
providers/v1/volcengine/auth_test.go

@@ -38,7 +38,7 @@ func TestNewSession_should_return_session_with_default_credentials_when_auth_is_
 	}
 	kube := fake.NewClientBuilder().Build()
 
-	sess, err := NewSession(context.Background(), store, kube, testNamespace)
+	sess, err := NewSession(context.Background(), store, kube, esv1.SecretStoreKind, testNamespace)
 
 	assert.NoError(t, err)
 	assert.NotNil(t, sess)
@@ -55,7 +55,7 @@ func TestNewSession_should_return_session_with_default_credentials_when_secretre
 	}
 	kube := fake.NewClientBuilder().Build()
 
-	sess, err := NewSession(context.Background(), store, kube, testNamespace)
+	sess, err := NewSession(context.Background(), store, kube, esv1.SecretStoreKind, testNamespace)
 
 	assert.NoError(t, err)
 	assert.NotNil(t, sess)
@@ -96,7 +96,7 @@ func TestNewSession_should_return_session_with_static_credentials_when_secretref
 	_ = v1.AddToScheme(scheme)
 	kube := fake.NewClientBuilder().WithScheme(scheme).WithObjects(secret).Build()
 
-	sess, err := NewSession(context.Background(), store, kube, testNamespace)
+	sess, err := NewSession(context.Background(), store, kube, esv1.SecretStoreKind, testNamespace)
 
 	assert.NoError(t, err)
 	assert.NotNil(t, sess)
@@ -121,7 +121,7 @@ func TestNewSession_should_return_error_when_accesskeyid_secret_is_not_found(t *
 	}
 	kube := fake.NewClientBuilder().Build()
 
-	_, err := NewSession(context.Background(), store, kube, testNamespace)
+	_, err := NewSession(context.Background(), store, kube, esv1.SecretStoreKind, testNamespace)
 
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "failed to get accessKeyID")
@@ -156,7 +156,7 @@ func TestNewSession_should_return_error_when_secretaccesskey_secret_is_not_found
 	_ = v1.AddToScheme(scheme)
 	kube := fake.NewClientBuilder().WithScheme(scheme).WithObjects(secret).Build()
 
-	_, err := NewSession(context.Background(), store, kube, testNamespace)
+	_, err := NewSession(context.Background(), store, kube, esv1.SecretStoreKind, testNamespace)
 
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "failed to get secretAccessKey")
@@ -185,10 +185,10 @@ func TestNewSession_should_return_error_when_accesskeyid_key_is_not_found_in_sec
 	_ = v1.AddToScheme(scheme)
 	kube := fake.NewClientBuilder().WithScheme(scheme).WithObjects(secret).Build()
 
-	_, err := NewSession(context.Background(), store, kube, testNamespace)
+	_, err := NewSession(context.Background(), store, kube, esv1.SecretStoreKind, testNamespace)
 
 	assert.Error(t, err)
-	assert.Contains(t, err.Error(), "key \"non-existent-key\" not found in secret")
+	assert.Contains(t, err.Error(), "cannot find secret data for key: \"non-existent-key\"")
 }
 
 func TestNewSession_should_return_session_with_token_credentials_when_secretref_is_provided(t *testing.T) {
@@ -227,7 +227,7 @@ func TestNewSession_should_return_session_with_token_credentials_when_secretref_
 	_ = v1.AddToScheme(scheme)
 	kube := fake.NewClientBuilder().WithScheme(scheme).WithObjects(secret).Build()
 
-	sess, err := NewSession(context.Background(), store, kube, testNamespace)
+	sess, err := NewSession(context.Background(), store, kube, esv1.SecretStoreKind, testNamespace)
 
 	assert.NoError(t, err)
 	assert.NotNil(t, sess)
@@ -268,10 +268,10 @@ func TestNewSession_should_return_error_when_secretaccesskey_key_is_not_found_in
 	_ = v1.AddToScheme(scheme)
 	kube := fake.NewClientBuilder().WithScheme(scheme).WithObjects(secret).Build()
 
-	_, err := NewSession(context.Background(), store, kube, testNamespace)
+	_, err := NewSession(context.Background(), store, kube, esv1.SecretStoreKind, testNamespace)
 
 	assert.Error(t, err)
-	assert.Contains(t, err.Error(), "key \"non-existent-key\" not found in secret")
+	assert.Contains(t, err.Error(), "cannot find secret data for key: \"non-existent-key\"")
 }
 
 func TestNewSession_should_return_error_when_store_is_nil(t *testing.T) {
@@ -280,7 +280,7 @@ func TestNewSession_should_return_error_when_store_is_nil(t *testing.T) {
 	var kube client.Client
 	namespace := "default"
 
-	sess, err := NewSession(ctx, store, kube, namespace)
+	sess, err := NewSession(ctx, store, kube, esv1.SecretStoreKind, namespace)
 
 	assert.Error(t, err)
 	assert.Nil(t, sess)
@@ -293,9 +293,95 @@ func TestNewSession_should_return_error_when_region_is_empty(t *testing.T) {
 	var kube client.Client
 	namespace := "default"
 
-	sess, err := NewSession(ctx, store, kube, namespace)
+	sess, err := NewSession(ctx, store, kube, esv1.SecretStoreKind, namespace)
 
 	assert.Error(t, err)
 	assert.Nil(t, sess)
 	assert.Equal(t, "region must be specified", err.Error())
 }
+
+func TestNewSession_ClusterSecretStore_should_resolve_secret_from_ref_namespace(t *testing.T) {
+	const credentialsNamespace = "credentials-ns"
+	secret := &v1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      secretName,
+			Namespace: credentialsNamespace,
+		},
+		Data: map[string][]byte{
+			accessKeyIDKey:     []byte(testAccessKeyID),
+			secretAccessKeyKey: []byte(testSecretAccessKey),
+		},
+	}
+	refNamespace := credentialsNamespace
+	store := &esv1.VolcengineProvider{
+		Region: testRegion,
+		Auth: &esv1.VolcengineAuth{
+			SecretRef: &esv1.VolcengineAuthSecretRef{
+				AccessKeyID: esmeta.SecretKeySelector{
+					Name:      secretName,
+					Key:       accessKeyIDKey,
+					Namespace: &refNamespace,
+				},
+				SecretAccessKey: esmeta.SecretKeySelector{
+					Name:      secretName,
+					Key:       secretAccessKeyKey,
+					Namespace: &refNamespace,
+				},
+			},
+		},
+	}
+
+	scheme := runtime.NewScheme()
+	_ = v1.AddToScheme(scheme)
+	kube := fake.NewClientBuilder().WithScheme(scheme).WithObjects(secret).Build()
+
+	sess, err := NewSession(context.Background(), store, kube, esv1.ClusterSecretStoreKind, testNamespace)
+
+	assert.NoError(t, err)
+	assert.NotNil(t, sess)
+	creds, err := sess.Config.Credentials.Get()
+	assert.NoError(t, err)
+	assert.Equal(t, testAccessKeyID, creds.AccessKeyID)
+	assert.Equal(t, testSecretAccessKey, creds.SecretAccessKey)
+}
+
+func TestNewSession_SecretStore_should_ignore_ref_namespace(t *testing.T) {
+	const otherNamespace = "other-ns"
+	secret := &v1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      secretName,
+			Namespace: otherNamespace,
+		},
+		Data: map[string][]byte{
+			accessKeyIDKey:     []byte(testAccessKeyID),
+			secretAccessKeyKey: []byte(testSecretAccessKey),
+		},
+	}
+	refNamespace := otherNamespace
+	store := &esv1.VolcengineProvider{
+		Region: testRegion,
+		Auth: &esv1.VolcengineAuth{
+			SecretRef: &esv1.VolcengineAuthSecretRef{
+				AccessKeyID: esmeta.SecretKeySelector{
+					Name:      secretName,
+					Key:       accessKeyIDKey,
+					Namespace: &refNamespace,
+				},
+				SecretAccessKey: esmeta.SecretKeySelector{
+					Name:      secretName,
+					Key:       secretAccessKeyKey,
+					Namespace: &refNamespace,
+				},
+			},
+		},
+	}
+
+	scheme := runtime.NewScheme()
+	_ = v1.AddToScheme(scheme)
+	kube := fake.NewClientBuilder().WithScheme(scheme).WithObjects(secret).Build()
+
+	_, err := NewSession(context.Background(), store, kube, esv1.SecretStoreKind, testNamespace)
+
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "failed to get accessKeyID")
+}

+ 1 - 1
providers/v1/volcengine/provider.go

@@ -41,7 +41,7 @@ func (p *Provider) NewClient(ctx context.Context, store esv1.GenericStore, kube
 		return nil, err
 	}
 
-	sess, err := NewSession(ctx, volcengineProvider, kube, namespace)
+	sess, err := NewSession(ctx, volcengineProvider, kube, store.GetKind(), namespace)
 	if err != nil {
 		return nil, err
 	}