fix: Skip Resolving Secrets From Restricted Namespace (#4735)

Signed-off-by: Indresh-Prakash <[email protected]>

CHANGELOG.md

@@ -56,6 +56,7 @@ To learn more about active deprecations, we recommend checking [GitHub Discussio
 ### Fixes
 
 - **General**: Paused ScaledObject continues working after removing the annotation ([#4733](https://github.com/kedacore/keda/issues/4733))
+- **General**: Skip resolving secrets if namespace is restricted ([#4519](https://github.com/kedacore/keda/issues/4519))
 
 ### Deprecations
 

Makefile

@@ -163,14 +163,17 @@ proto-gen: protoc-gen ## Generate Liiklus, ExternalScaler and MetricsService pro
 	PATH="$(LOCALBIN):$(PATH)" protoc -I vendor --proto_path=pkg/metricsservice/api metrics.proto --go_out=pkg/metricsservice/api --go-grpc_out=pkg/metricsservice/api
 
 .PHONY: mockgen-gen
-mockgen-gen: mockgen pkg/mock/mock_scaling/mock_interface.go pkg/mock/mock_scaling/mock_executor/mock_interface.go pkg/mock/mock_scaler/mock_scaler.go pkg/mock/mock_scale/mock_interfaces.go pkg/mock/mock_client/mock_interfaces.go pkg/scalers/liiklus/mocks/mock_liiklus.go
+mockgen-gen: mockgen pkg/mock/mock_scaling/mock_interface.go pkg/mock/mock_scaling/mock_executor/mock_interface.go pkg/mock/mock_scaler/mock_scaler.go pkg/mock/mock_scale/mock_interfaces.go pkg/mock/mock_client/mock_interfaces.go pkg/scalers/liiklus/mocks/mock_liiklus.go pkg/mock/mock_secretlister/mock_interfaces.go
 
 pkg/mock/mock_scaling/mock_interface.go: pkg/scaling/scale_handler.go
 	$(MOCKGEN) -destination=$@ -package=mock_scaling -source=$^
 pkg/mock/mock_scaling/mock_executor/mock_interface.go: pkg/scaling/executor/scale_executor.go
 	$(MOCKGEN) -destination=$@ -package=mock_executor -source=$^
 pkg/mock/mock_scaler/mock_scaler.go: pkg/scalers/scaler.go
 	$(MOCKGEN) -destination=$@ -package=mock_scalers -source=$^
+pkg/mock/mock_secretlister/mock_interfaces.go: vendor/k8s.io/client-go/listers/core/v1/secret.go
+	mkdir -p pkg/mock/mock_secretlister
+	$(MOCKGEN) k8s.io/client-go/listers/core/v1 SecretLister,SecretNamespaceLister > $@
 pkg/mock/mock_scale/mock_interfaces.go: vendor/k8s.io/client-go/scale/interfaces.go
 	mkdir -p pkg/mock/mock_scale
 	$(MOCKGEN) k8s.io/client-go/scale ScalesGetter,ScaleInterface > $@

pkg/scaling/resolver/scale_resolvers.go

@@ -20,6 +20,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"strconv"
 	"strings"
 
 	"github.com/go-logr/logr"
@@ -41,6 +42,8 @@ const (
 	referenceOperator = '$'
 	referenceOpener   = '('
 	referenceCloser   = ')'
+	boolTrue          = true
+	boolFalse         = false
 )
 
 var (
@@ -52,13 +55,13 @@ var (
 // isSecretAccessRestricted returns whether secret access need to be restricted in KEDA namespace
 func isSecretAccessRestricted(logger logr.Logger) bool {
 	if restrictSecretAccess == "" {
-		return false
+		return boolFalse
 	}
-	if strings.ToLower(restrictSecretAccess) == "true" {
+	if strings.ToLower(restrictSecretAccess) == strconv.FormatBool(boolTrue) {
 		logger.V(1).Info("Secret Access is restricted to be in Cluster Object Namespace, please use ClusterTriggerAuthentication instead of TriggerAuthentication", "Cluster Object Namespace", kedaNamespace, "Env Var", util.RestrictSecretAccessEnvVar, "Env Value", strings.ToLower(restrictSecretAccess))
-		return true
+		return boolTrue
 	}
-	return false
+	return boolFalse
 }
 
 // ResolveScaleTargetPodSpec for given scalableObject inspects the scale target workload,
@@ -151,11 +154,11 @@ func ResolveContainerEnv(ctx context.Context, client client.Client, logger logr.
 
 	var container corev1.Container
 	if containerName != "" {
-		containerWithNameFound := false
+		containerWithNameFound := boolFalse
 		for _, c := range podSpec.Containers {
 			if c.Name == containerName {
 				container = c
-				containerWithNameFound = true
+				containerWithNameFound = boolTrue
 				break
 			}
 		}
@@ -304,7 +307,8 @@ func getTriggerAuthSpec(ctx context.Context, client client.Client, triggerAuthRe
 
 func resolveEnv(ctx context.Context, client client.Client, logger logr.Logger, container *corev1.Container, namespace string, secretsLister corev1listers.SecretLister) (map[string]string, error) {
 	resolved := make(map[string]string)
-
+	secretAccessRestricted := isSecretAccessRestricted(logger)
+	accessSecrets := readSecrets(secretAccessRestricted, namespace)
 	if container.EnvFrom != nil {
 		for _, source := range container.EnvFrom {
 			if source.ConfigMapRef != nil {
@@ -320,7 +324,7 @@ func resolveEnv(ctx context.Context, client client.Client, logger logr.Logger, c
 				default:
 					return nil, fmt.Errorf("error reading config ref %s on namespace %s/: %w", source.ConfigMapRef, namespace, err)
 				}
-			} else if source.SecretRef != nil {
+			} else if source.SecretRef != nil && accessSecrets {
 				secretsMap, err := resolveSecretMap(ctx, client, logger, source.SecretRef, namespace, secretsLister)
 				switch {
 				case err == nil:
@@ -349,7 +353,7 @@ func resolveEnv(ctx context.Context, client client.Client, logger logr.Logger, c
 			} else if envVar.ValueFrom != nil {
 				// env is an EnvVarSource, that can be on of the 4 below
 				switch {
-				case envVar.ValueFrom.SecretKeyRef != nil:
+				case envVar.ValueFrom.SecretKeyRef != nil && accessSecrets:
 					// env is a secret selector
 					value, err = resolveSecretValue(ctx, client, logger, envVar.ValueFrom.SecretKeyRef, envVar.ValueFrom.SecretKeyRef.Key, namespace, secretsLister)
 					if err != nil {
@@ -384,6 +388,13 @@ func resolveEnv(ctx context.Context, client client.Client, logger logr.Logger, c
 	return resolved, nil
 }
 
+func readSecrets(secretAccessRestricted bool, namespace string) bool {
+	if secretAccessRestricted && (namespace != kedaNamespace) {
+		return boolFalse
+	}
+	return boolTrue
+}
+
 func resolveEnvValue(value string, env map[string]string) string {
 	var buf bytes.Buffer
 	checkpoint := 0

pkg/scaling/resolver/scale_resolvers_test.go

@@ -21,6 +21,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/golang/mock/gomock"
 	"github.com/google/go-cmp/cmp"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -31,6 +32,7 @@ import (
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
 	kedav1alpha1 "github.com/kedacore/keda/v2/apis/keda/v1alpha1"
+	mock_v1 "github.com/kedacore/keda/v2/pkg/mock/mock_secretlister"
 )
 
 var (
@@ -541,3 +543,117 @@ func TestResolveDependentEnv(t *testing.T) {
 		})
 	}
 }
+
+func TestEnvWithRestrictSecretAccess(t *testing.T) {
+	tests := []struct {
+		name      string
+		expected  map[string]string
+		container *corev1.Container
+	}{
+		{
+			name: "env reference secret key",
+			container: &corev1.Container{
+				Env: []corev1.EnvVar{{
+					Name: envKey,
+					ValueFrom: &corev1.EnvVarSource{
+						SecretKeyRef: &corev1.SecretKeySelector{
+							LocalObjectReference: corev1.LocalObjectReference{
+								Name: secretName,
+							},
+							Key: secretKey,
+						},
+					},
+				}},
+			},
+			expected: map[string]string{},
+		},
+		{
+			name: "env reference secret name",
+			container: &corev1.Container{
+				EnvFrom: []corev1.EnvFromSource{{
+					SecretRef: &corev1.SecretEnvSource{
+						LocalObjectReference: corev1.LocalObjectReference{
+							Name: secretName,
+						},
+					},
+				}},
+			},
+			expected: map[string]string{},
+		},
+	}
+	var secretsLister corev1listers.SecretLister
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			restrictSecretAccess = "true"
+			ctx := context.Background()
+			envMap, _ := resolveEnv(ctx, fake.NewClientBuilder().Build(), logf.Log.WithName("test"), test.container, namespace, secretsLister)
+			if diff := cmp.Diff(envMap, test.expected); diff != "" {
+				t.Errorf("Returned env map is different: %s", diff)
+			}
+		})
+	}
+}
+
+func TestEnvWithRestrictedNamespace(t *testing.T) {
+	tests := []struct {
+		name      string
+		expected  map[string]string
+		container *corev1.Container
+	}{
+		{
+			name: "env reference secret key",
+			container: &corev1.Container{
+				Env: []corev1.EnvVar{{
+					Name: envKey,
+					ValueFrom: &corev1.EnvVarSource{
+						SecretKeyRef: &corev1.SecretKeySelector{
+							LocalObjectReference: corev1.LocalObjectReference{
+								Name: secretName,
+							},
+							Key: secretKey,
+						},
+					},
+				}},
+			},
+			expected: map[string]string{envKey: secretData},
+		},
+		{
+			name: "env reference secret name",
+			container: &corev1.Container{
+				EnvFrom: []corev1.EnvFromSource{{
+					SecretRef: &corev1.SecretEnvSource{
+						LocalObjectReference: corev1.LocalObjectReference{
+							Name: secretName,
+						},
+					},
+				}},
+			},
+			expected: map[string]string{secretKey: secretData},
+		},
+	}
+	secret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: clusterNamespace,
+			Name:      secretName,
+		},
+		Data: map[string][]byte{secretKey: []byte(secretData)},
+	}
+	ctrl := gomock.NewController(t)
+	mockSecretNamespaceLister := mock_v1.NewMockSecretNamespaceLister(ctrl)
+	mockSecretNamespaceLister.EXPECT().Get(secretName).Return(secret, nil).AnyTimes()
+	mockSecretLister := mock_v1.NewMockSecretLister(ctrl)
+	mockSecretLister.EXPECT().Secrets(clusterNamespace).Return(mockSecretNamespaceLister).AnyTimes()
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			restrictSecretAccess = "true"
+			kedaNamespace = "keda"
+			ctx := context.Background()
+			envMap, _ := resolveEnv(ctx, fake.NewClientBuilder().Build(), logf.Log.WithName("test"), test.container, clusterNamespace, mockSecretLister)
+			if diff := cmp.Diff(envMap, test.expected); diff != "" {
+				t.Errorf("Returned env map is different: %s", diff)
+			}
+		})
+	}
+}