diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index fe7e22018ac..e357acbd6ab 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -37,7 +37,6 @@ RUN apt-get update \ github.com/ramya-rao-a/go-outline \ github.com/acroca/go-symbols \ github.com/godoctor/godoctor \ - golang.org/x/tools/cmd/guru \ golang.org/x/tools/cmd/gorename \ github.com/rogpeppe/godef \ github.com/zmb3/gogetdoc \ diff --git a/CHANGELOG.md b/CHANGELOG.md index 01af46e3c0d..40e117fcd06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,10 +65,12 @@ Here is an overview of all new **experimental** features: - **General**: Add command-line flag in Adapter to allow override of gRPC Authority Header ([#5449](https://github.com/kedacore/keda/issues/5449)) - **General**: Add OPENTELEMETRY flag in e2e test YAML ([#5375](https://github.com/kedacore/keda/issues/5375)) +- **General**: Add support for cross tenant/cloud authentication when using Azure Workload Identity for TriggerAuthentication ([#5441](https://github.com/kedacore/keda/issues/5441)) +- **MongoDB Scaler**: Add scheme field support srv record ([#5544](https://github.com/kedacore/keda/issues/5544)) ### Fixes -- TODO ([#XXX](https://github.com/kedacore/keda/issues/XXX)) +- **General**: Validate empty array value of triggers in ScaledObject/ScaledJob creation ([#5520](https://github.com/kedacore/keda/issues/5520)) ### Deprecations @@ -84,10 +86,12 @@ New deprecation(s): ### Other +- **General**: Allow E2E tests to be run against existing KEDA and/or Kafka installation ([#5595](https://github.com/kedacore/keda/pull/5595)) - **General**: Improve readability of utility function getParameterFromConfigV2 ([#5037](https://github.com/kedacore/keda/issues/5037)) -- **General**: Introduce ENABLE_OPENTELEMETRY in deploying/testing process ([#5375](https://github.com/kedacore/keda/issues/5375)) +- **General**: Introduce ENABLE_OPENTELEMETRY in deploying/testing process ([#5375](https://github.com/kedacore/keda/issues/5375)|[#5578](https://github.com/kedacore/keda/issues/5578)) - **General**: Migrate away from unmaintained golang/mock and use uber/gomock ([#5440](https://github.com/kedacore/keda/issues/5440)) - **General**: Minor refactor to reduce copy/paste code in ScaledObject webhook ([#5397](https://github.com/kedacore/keda/issues/5397)) +- **Kafka**: Expose GSSAPI service name ([#5474](https://github.com/kedacore/keda/issues/5474)) ## v2.13.1 diff --git a/apis/keda/v1alpha1/scaledjob_webhook.go b/apis/keda/v1alpha1/scaledjob_webhook.go new file mode 100644 index 00000000000..24eb03d3f24 --- /dev/null +++ b/apis/keda/v1alpha1/scaledjob_webhook.go @@ -0,0 +1,73 @@ +/* +Copyright 2024 The KEDA Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + "encoding/json" + "fmt" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +var scaledjoblog = logf.Log.WithName("scaledjob-validation-webhook") + +func (s *ScaledJob) SetupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(s). + Complete() +} + +// +kubebuilder:webhook:path=/validate-keda-sh-v1alpha1-scaledjob,mutating=false,failurePolicy=ignore,sideEffects=None,groups=keda.sh,resources=scaledjobs,verbs=create;update,versions=v1alpha1,name=vscaledjob.kb.io,admissionReviewVersions=v1 + +var _ webhook.Validator = &ScaledJob{} + +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type +func (s *ScaledJob) ValidateCreate() (admission.Warnings, error) { + val, _ := json.MarshalIndent(s, "", " ") + scaledjoblog.Info(fmt.Sprintf("validating scaledjob creation for %s", string(val))) + return nil, verifyTriggers(s, "create", false) +} + +func (s *ScaledJob) ValidateUpdate(old runtime.Object) (admission.Warnings, error) { + val, _ := json.MarshalIndent(s, "", " ") + scaledobjectlog.V(1).Info(fmt.Sprintf("validating scaledjob update for %s", string(val))) + + oldTa := old.(*ScaledJob) + if isScaledJobRemovingFinalizer(s.ObjectMeta, oldTa.ObjectMeta, s.Spec, oldTa.Spec) { + scaledjoblog.V(1).Info("finalizer removal, skipping validation") + return nil, nil + } + return nil, verifyTriggers(s, "update", false) +} + +func (s *ScaledJob) ValidateDelete() (admission.Warnings, error) { + return nil, nil +} + +func isScaledJobRemovingFinalizer(om metav1.ObjectMeta, oldOm metav1.ObjectMeta, spec ScaledJobSpec, oldSpec ScaledJobSpec) bool { + taSpec, _ := json.MarshalIndent(spec, "", " ") + oldTaSpec, _ := json.MarshalIndent(oldSpec, "", " ") + taSpecString := string(taSpec) + oldTaSpecString := string(oldTaSpec) + + return len(om.Finalizers) == 0 && len(oldOm.Finalizers) == 1 && taSpecString == oldTaSpecString +} diff --git a/apis/keda/v1alpha1/scaledjob_webhook_test.go b/apis/keda/v1alpha1/scaledjob_webhook_test.go new file mode 100644 index 00000000000..408495ff893 --- /dev/null +++ b/apis/keda/v1alpha1/scaledjob_webhook_test.go @@ -0,0 +1,83 @@ +/* +Copyright 2024 The KEDA Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var _ = It("should validate empty triggers in ScaledJob", func() { + + namespaceName := "scaledjob-empty-triggers-set" + namespace := createNamespace(namespaceName) + + err := k8sClient.Create(context.Background(), namespace) + Expect(err).ToNot(HaveOccurred()) + + sj := createScaledJob(sjName, namespaceName, []ScaleTriggers{}) + + Eventually(func() error { + return k8sClient.Create(context.Background(), sj) + }).Should(HaveOccurred()) +}) + +// -------------------------------------------------------------------------- // +// ----------------------------- HELP FUNCTIONS ----------------------------- // +// -------------------------------------------------------------------------- // +func createScaledJob(name string, namespace string, triggers []ScaleTriggers) *ScaledJob { + return &ScaledJob{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + TypeMeta: metav1.TypeMeta{ + Kind: "ScaledJob", + APIVersion: "keda.sh", + }, + Spec: ScaledJobSpec{ + JobTargetRef: &batchv1.JobSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": name, + }, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": name, + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: name, + Image: name, + }, + }, + }, + }, + }, + Triggers: triggers, + }, + } +} diff --git a/apis/keda/v1alpha1/scaledobject_webhook.go b/apis/keda/v1alpha1/scaledobject_webhook.go index e18b59f1fca..4c8253f640c 100644 --- a/apis/keda/v1alpha1/scaledobject_webhook.go +++ b/apis/keda/v1alpha1/scaledobject_webhook.go @@ -132,7 +132,6 @@ func validateWorkload(so *ScaledObject, action string, dryRun bool) (admission.W verifyFunctions := []func(*ScaledObject, string, bool) error{ verifyCPUMemoryScalers, - verifyTriggers, verifyScaledObjects, verifyHpas, verifyReplicaCount, @@ -145,6 +144,17 @@ func validateWorkload(so *ScaledObject, action string, dryRun bool) (admission.W } } + verifyCommonFunctions := []func(interface{}, string, bool) error{ + verifyTriggers, + } + + for i := range verifyCommonFunctions { + err := verifyCommonFunctions[i](so, action, dryRun) + if err != nil { + return nil, err + } + } + scaledobjectlog.V(1).Info(fmt.Sprintf("scaledobject %s is valid", so.Name)) return nil, nil } @@ -158,11 +168,27 @@ func verifyReplicaCount(incomingSo *ScaledObject, action string, _ bool) error { return nil } -func verifyTriggers(incomingSo *ScaledObject, action string, _ bool) error { - err := ValidateTriggers(incomingSo.Spec.Triggers) +func verifyTriggers(incomingObject interface{}, action string, _ bool) error { + var triggers []ScaleTriggers + var name string + var namespace string + switch obj := incomingObject.(type) { + case *ScaledObject: + triggers = obj.Spec.Triggers + name = obj.Name + namespace = obj.Namespace + case *ScaledJob: + triggers = obj.Spec.Triggers + name = obj.Name + namespace = obj.Namespace + default: + return fmt.Errorf("unknown scalable object type %v", incomingObject) + } + + err := ValidateTriggers(triggers) if err != nil { - scaledobjectlog.WithValues("name", incomingSo.Name).Error(err, "validation error") - metricscollector.RecordScaledObjectValidatingErrors(incomingSo.Namespace, action, "incorrect-triggers") + scaledobjectlog.WithValues("name", name).Error(err, "validation error") + metricscollector.RecordScaledObjectValidatingErrors(namespace, action, "incorrect-triggers") } return err } diff --git a/apis/keda/v1alpha1/scaledobject_webhook_test.go b/apis/keda/v1alpha1/scaledobject_webhook_test.go index 2edfe24f07a..be9640f319f 100644 --- a/apis/keda/v1alpha1/scaledobject_webhook_test.go +++ b/apis/keda/v1alpha1/scaledobject_webhook_test.go @@ -583,6 +583,30 @@ var _ = It("shouldn't create so when stabilizationWindowSeconds exceeds 3600", f Should(HaveOccurred()) }) +var _ = It("should validate empty triggers in ScaledObject", func() { + + namespaceName := "empty-triggers-set" + namespace := createNamespace(namespaceName) + + err := k8sClient.Create(context.Background(), namespace) + Expect(err).ToNot(HaveOccurred()) + + workload := createDeployment(namespaceName, false, false) + workload.Spec.Template.Spec.Containers[0].Resources.Limits = v1.ResourceList{ + v1.ResourceCPU: *resource.NewMilliQuantity(100, resource.DecimalSI), + } + + err = k8sClient.Create(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + + so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "") + so.Spec.Triggers = []ScaleTriggers{} + + Eventually(func() error { + return k8sClient.Create(context.Background(), so) + }).Should(HaveOccurred()) +}) + // ============================ SCALING MODIFIERS ============================ \\ // =========================================================================== \\ diff --git a/apis/keda/v1alpha1/scaletriggers_types.go b/apis/keda/v1alpha1/scaletriggers_types.go index 2e8afb6e4fd..584dfa8b614 100644 --- a/apis/keda/v1alpha1/scaletriggers_types.go +++ b/apis/keda/v1alpha1/scaletriggers_types.go @@ -51,6 +51,11 @@ type AuthenticationRef struct { // - useCachedMetrics is defined only for a supported triggers func ValidateTriggers(triggers []ScaleTriggers) error { triggersCount := len(triggers) + + if triggersCount == 0 { + return fmt.Errorf("no triggers defined in the ScaledObject/ScaledJob") + } + if triggers != nil && triggersCount > 0 { triggerNames := make(map[string]bool, triggersCount) for i := 0; i < triggersCount; i++ { @@ -66,7 +71,7 @@ func ValidateTriggers(triggers []ScaleTriggers) error { if name != "" { if _, found := triggerNames[name]; found { // found duplicate name - return fmt.Errorf("triggerName %q is defined multiple times in the ScaledObject, but it must be unique", name) + return fmt.Errorf("triggerName %q is defined multiple times in the ScaledObject/ScaledJob, but it must be unique", name) } triggerNames[name] = true } diff --git a/apis/keda/v1alpha1/scaletriggers_types_test.go b/apis/keda/v1alpha1/scaletriggers_types_test.go index 75c8babc865..cc140df69cb 100644 --- a/apis/keda/v1alpha1/scaletriggers_types_test.go +++ b/apis/keda/v1alpha1/scaletriggers_types_test.go @@ -38,7 +38,7 @@ func TestValidateTriggers(t *testing.T) { Type: "prometheus", }, }, - expectedErrMsg: "triggerName \"trigger1\" is defined multiple times in the ScaledObject, but it must be unique", + expectedErrMsg: "triggerName \"trigger1\" is defined multiple times in the ScaledObject/ScaledJob, but it must be unique", }, { name: "unsupported useCachedMetrics property for cpu scaler", @@ -84,6 +84,11 @@ func TestValidateTriggers(t *testing.T) { }, expectedErrMsg: "", }, + { + name: "empty triggers array should be blocked", + triggers: []ScaleTriggers{}, + expectedErrMsg: "no triggers defined in the ScaledObject/ScaledJob", + }, } for _, test := range tests { diff --git a/apis/keda/v1alpha1/suite_test.go b/apis/keda/v1alpha1/suite_test.go index 2a9e69195fe..ce53cc62682 100644 --- a/apis/keda/v1alpha1/suite_test.go +++ b/apis/keda/v1alpha1/suite_test.go @@ -52,6 +52,7 @@ var cancel context.CancelFunc const ( workloadName = "deployment-name" soName = "test-so" + sjName = "test-sj" ) func TestAPIs(t *testing.T) { @@ -119,6 +120,8 @@ var _ = BeforeSuite(func() { err = (&ScaledObject{}).SetupWebhookWithManager(mgr) Expect(err).NotTo(HaveOccurred()) + err = (&ScaledJob{}).SetupWebhookWithManager(mgr) + Expect(err).NotTo(HaveOccurred()) err = (&TriggerAuthentication{}).SetupWebhookWithManager(mgr) Expect(err).NotTo(HaveOccurred()) err = (&ClusterTriggerAuthentication{}).SetupWebhookWithManager(mgr) diff --git a/apis/keda/v1alpha1/triggerauthentication_types.go b/apis/keda/v1alpha1/triggerauthentication_types.go index 61d1c19d65f..3f283351057 100644 --- a/apis/keda/v1alpha1/triggerauthentication_types.go +++ b/apis/keda/v1alpha1/triggerauthentication_types.go @@ -139,13 +139,24 @@ const ( // AuthPodIdentity allows users to select the platform native identity // mechanism type AuthPodIdentity struct { - // +kubebuilder:validation:Enum=azure;azure-workload;gcp;aws;aws-eks;aws-kiam + // +kubebuilder:validation:Enum=azure;azure-workload;gcp;aws;aws-eks;aws-kiam;none Provider PodIdentityProvider `json:"provider"` + // +optional IdentityID *string `json:"identityId"` + + // +optional + // Set identityTenantId to override the default Azure tenant id. If this is set, then the IdentityID must also be set + IdentityTenantID *string `json:"identityTenantId"` + // +optional + // Set identityAuthorityHost to override the default Azure authority host. If this is set, then the IdentityTenantID must also be set + IdentityAuthorityHost *string `json:"identityAuthorityHost"` + + // +kubebuilder:validation:Optional // RoleArn sets the AWS RoleArn to be used. Mutually exclusive with IdentityOwner - RoleArn string `json:"roleArn"` + RoleArn *string `json:"roleArn"` + // +kubebuilder:validation:Enum=keda;workload // +optional // IdentityOwner configures which identity has to be used during auto discovery, keda or the scaled workload. Mutually exclusive with roleArn @@ -159,6 +170,20 @@ func (a *AuthPodIdentity) GetIdentityID() string { return *a.IdentityID } +func (a *AuthPodIdentity) GetIdentityTenantID() string { + if a.IdentityTenantID == nil { + return "" + } + return *a.IdentityTenantID +} + +func (a *AuthPodIdentity) GetIdentityAuthorityHost() string { + if a.IdentityAuthorityHost == nil { + return "" + } + return *a.IdentityAuthorityHost +} + func (a *AuthPodIdentity) IsWorkloadIdentityOwner() bool { if a.IdentityOwner == nil { return false diff --git a/apis/keda/v1alpha1/triggerauthentication_webhook.go b/apis/keda/v1alpha1/triggerauthentication_webhook.go index df77bbc1e18..f77452e2473 100644 --- a/apis/keda/v1alpha1/triggerauthentication_webhook.go +++ b/apis/keda/v1alpha1/triggerauthentication_webhook.go @@ -116,10 +116,18 @@ func validateSpec(spec *TriggerAuthenticationSpec) (admission.Warnings, error) { switch spec.PodIdentity.Provider { case PodIdentityProviderAzure, PodIdentityProviderAzureWorkload: if spec.PodIdentity.IdentityID != nil && *spec.PodIdentity.IdentityID == "" { - return nil, fmt.Errorf("identityid of PodIdentity should not be empty. If it's set, identityId has to be different than \"\"") + return nil, fmt.Errorf("identityId of PodIdentity should not be empty. If it's set, identityId has to be different than \"\"") + } + + if spec.PodIdentity.IdentityAuthorityHost != nil && *spec.PodIdentity.IdentityAuthorityHost != "" { + if spec.PodIdentity.IdentityTenantID == nil || *spec.PodIdentity.IdentityTenantID == "" { + return nil, fmt.Errorf("identityTenantID of PodIdentity should not be nil or empty when identityAuthorityHost of PodIdentity is set") + } + } else if spec.PodIdentity.IdentityTenantID != nil && *spec.PodIdentity.IdentityTenantID == "" { + return nil, fmt.Errorf("identityTenantId of PodIdentity should not be empty. If it's set, identityTenantId has to be different than \"\"") } case PodIdentityProviderAws: - if spec.PodIdentity.RoleArn != "" && spec.PodIdentity.IsWorkloadIdentityOwner() { + if spec.PodIdentity.RoleArn != nil && *spec.PodIdentity.RoleArn != "" && spec.PodIdentity.IsWorkloadIdentityOwner() { return nil, fmt.Errorf("roleArn of PodIdentity can't be set if KEDA isn't identityOwner") } default: diff --git a/apis/keda/v1alpha1/triggerauthentication_webhook_test.go b/apis/keda/v1alpha1/triggerauthentication_webhook_test.go index 44ea8ed762c..8c984915ba8 100644 --- a/apis/keda/v1alpha1/triggerauthentication_webhook_test.go +++ b/apis/keda/v1alpha1/triggerauthentication_webhook_test.go @@ -30,7 +30,7 @@ var _ = It("validate triggerauthentication when IdentityID is nil, roleArn is em err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) - spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAzure, "", nil, nil) + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAzure, nil, nil, nil, nil, nil) ta := createTriggerAuthentication("nilidentityidta", namespaceName, "TriggerAuthentication", spec) Eventually(func() error { return k8sClient.Create(context.Background(), ta) @@ -44,7 +44,7 @@ var _ = It("validate triggerauthentication when IdentityID is empty", func() { Expect(err).ToNot(HaveOccurred()) identityID := "" - spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAzure, "", &identityID, nil) + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAzure, nil, &identityID, nil, nil, nil) ta := createTriggerAuthentication("emptyidentityidta", namespaceName, "TriggerAuthentication", spec) Eventually(func() error { return k8sClient.Create(context.Background(), ta) @@ -58,20 +58,98 @@ var _ = It("validate triggerauthentication when IdentityID is not empty", func() Expect(err).ToNot(HaveOccurred()) identityID := "12345" - spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAzure, "", &identityID, nil) + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAzure, nil, &identityID, nil, nil, nil) ta := createTriggerAuthentication("identityidta", namespaceName, "TriggerAuthentication", spec) Eventually(func() error { return k8sClient.Create(context.Background(), ta) }).ShouldNot(HaveOccurred()) }) +var _ = It("validate triggerauthentication when IdentityTenantID is not nil and not empty", func() { + namespaceName := "identitytenantidta" + namespace := createNamespace(namespaceName) + err := k8sClient.Create(context.Background(), namespace) + Expect(err).ToNot(HaveOccurred()) + + identityID := "12345" + identityTenantID := "12345" + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAzureWorkload, nil, &identityID, &identityTenantID, nil, nil) + ta := createTriggerAuthentication("identitytenantidta", namespaceName, "TriggerAuthentication", spec) + Eventually(func() error { + return k8sClient.Create(context.Background(), ta) + }).ShouldNot(HaveOccurred()) +}) + +var _ = It("validate triggerauthentication when IdentityTenantID is not nil but empty", func() { + namespaceName := "emptyidentitytenantidta" + namespace := createNamespace(namespaceName) + err := k8sClient.Create(context.Background(), namespace) + Expect(err).ToNot(HaveOccurred()) + + identityID := "12345" + identityTenantID := "" + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAzureWorkload, nil, &identityID, &identityTenantID, nil, nil) + ta := createTriggerAuthentication("emptyidentitytenantidta", namespaceName, "TriggerAuthentication", spec) + Eventually(func() error { + return k8sClient.Create(context.Background(), ta) + }).Should(HaveOccurred()) +}) + +var _ = It("validate triggerauthentication when IdentityAuthorityHost is not nil and not empty and IdentityTenantID is not nil and not empty", func() { + namespaceName := "identityauthorityhostta" + namespace := createNamespace(namespaceName) + err := k8sClient.Create(context.Background(), namespace) + Expect(err).ToNot(HaveOccurred()) + + identityID := "12345" + identityTenantID := "12345" + identityAuthorityHost := "12345" + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAzureWorkload, nil, &identityID, &identityTenantID, &identityAuthorityHost, nil) + ta := createTriggerAuthentication("identityauthorityhostta", namespaceName, "TriggerAuthentication", spec) + Eventually(func() error { + return k8sClient.Create(context.Background(), ta) + }).ShouldNot(HaveOccurred()) +}) + +var _ = It("validate triggerauthentication when IdentityAuthorityHost is not nil and not empty and IdentityTenantID is nil", func() { + namespaceName := "niltenantidentityauthorityhostta" + namespace := createNamespace(namespaceName) + err := k8sClient.Create(context.Background(), namespace) + Expect(err).ToNot(HaveOccurred()) + + identityID := "12345" + identityAuthorityHost := "12345" + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAzureWorkload, nil, &identityID, nil, &identityAuthorityHost, nil) + ta := createTriggerAuthentication("niltenantidentityauthorityhostta", namespaceName, "TriggerAuthentication", spec) + Eventually(func() error { + return k8sClient.Create(context.Background(), ta) + }).Should(HaveOccurred()) +}) + +var _ = It("validate triggerauthentication when IdentityAuthorityHost is not nil and not empty and IdentityTenantID is not nil but empty", func() { + namespaceName := "emptytenantidentityauthorityhostta" + namespace := createNamespace(namespaceName) + err := k8sClient.Create(context.Background(), namespace) + Expect(err).ToNot(HaveOccurred()) + + identityID := "12345" + identityTenantID := "" + identityAuthorityHost := "12345" + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAzureWorkload, nil, &identityID, &identityTenantID, &identityAuthorityHost, nil) + ta := createTriggerAuthentication("emptytenantidentityauthorityhostta", namespaceName, "TriggerAuthentication", spec) + Eventually(func() error { + return k8sClient.Create(context.Background(), ta) + }).Should(HaveOccurred()) +}) + var _ = It("validate triggerauthentication when RoleArn is not empty and IdentityOwner is nil", func() { namespaceName := "rolearn" namespace := createNamespace(namespaceName) err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) - spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAws, "Helo", nil, nil) + roleArn := "Hello" + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAws, &roleArn, nil, nil, nil, nil) ta := createTriggerAuthentication("identityidta", namespaceName, "TriggerAuthentication", spec) Eventually(func() error { return k8sClient.Create(context.Background(), ta) @@ -84,8 +162,9 @@ var _ = It("validate triggerauthentication when RoleArn is not empty and Identit err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) + roleArn := "Hello" identityOwner := kedaString - spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAws, "Helo", nil, &identityOwner) + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAws, &roleArn, nil, nil, nil, &identityOwner) ta := createTriggerAuthentication("identityidta", namespaceName, "TriggerAuthentication", spec) Eventually(func() error { return k8sClient.Create(context.Background(), ta) @@ -98,8 +177,9 @@ var _ = It("validate triggerauthentication when RoleArn is not empty and Identit err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) + roleArn := "Hello" identityOwner := workloadString - spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAws, "Helo", nil, &identityOwner) + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAws, &roleArn, nil, nil, nil, &identityOwner) ta := createTriggerAuthentication("identityidta", namespaceName, "TriggerAuthentication", spec) Eventually(func() error { return k8sClient.Create(context.Background(), ta) @@ -113,7 +193,7 @@ var _ = It("validate triggerauthentication when RoleArn is empty and IdentityOwn Expect(err).ToNot(HaveOccurred()) identityOwner := kedaString - spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAws, "", nil, &identityOwner) + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAws, nil, nil, nil, nil, &identityOwner) ta := createTriggerAuthentication("identityidta", namespaceName, "TriggerAuthentication", spec) Eventually(func() error { return k8sClient.Create(context.Background(), ta) @@ -127,7 +207,7 @@ var _ = It("validate triggerauthentication when RoleArn is not empty and Identit Expect(err).ToNot(HaveOccurred()) identityOwner := workloadString - spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAws, "", nil, &identityOwner) + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAws, nil, nil, nil, nil, &identityOwner) ta := createTriggerAuthentication("identityidta", namespaceName, "TriggerAuthentication", spec) Eventually(func() error { return k8sClient.Create(context.Background(), ta) @@ -140,7 +220,7 @@ var _ = It("validate clustertriggerauthentication when IdentityID is nil", func( err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) - spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAzure, "", nil, nil) + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAzure, nil, nil, nil, nil, nil) ta := createTriggerAuthentication("clusternilidentityidta", namespaceName, "ClusterTriggerAuthentication", spec) Eventually(func() error { return k8sClient.Create(context.Background(), ta) @@ -154,7 +234,7 @@ var _ = It("validate clustertriggerauthentication when IdentityID is empty", fun Expect(err).ToNot(HaveOccurred()) identityID := "" - spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAzure, "", &identityID, nil) + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAzure, nil, &identityID, nil, nil, nil) ta := createTriggerAuthentication("clusteremptyidentityidta", namespaceName, "ClusterTriggerAuthentication", spec) Eventually(func() error { return k8sClient.Create(context.Background(), ta) @@ -168,7 +248,7 @@ var _ = It("validate clustertriggerauthentication when IdentityID is not empty", Expect(err).ToNot(HaveOccurred()) identityID := "12345" - spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAzure, "", &identityID, nil) + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAzure, nil, &identityID, nil, nil, nil) ta := createTriggerAuthentication("clusteridentityidta", namespaceName, "ClusterTriggerAuthentication", spec) Eventually(func() error { return k8sClient.Create(context.Background(), ta) @@ -181,7 +261,8 @@ var _ = It("validate clustertriggerauthentication when RoleArn is not empty and err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) - spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAws, "Helo", nil, nil) + roleArn := "Hello" + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAws, &roleArn, nil, nil, nil, nil) ta := createTriggerAuthentication("clusteridentityidta", namespaceName, "ClusterTriggerAuthentication", spec) Eventually(func() error { return k8sClient.Create(context.Background(), ta) @@ -194,8 +275,9 @@ var _ = It("validate clustertriggerauthentication when RoleArn is not empty and err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) + roleArn := "Hello" identityOwner := kedaString - spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAws, "Helo", nil, &identityOwner) + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAws, &roleArn, nil, nil, nil, &identityOwner) ta := createTriggerAuthentication("clusteridentityidta", namespaceName, "ClusterTriggerAuthentication", spec) Eventually(func() error { return k8sClient.Create(context.Background(), ta) @@ -208,8 +290,9 @@ var _ = It("validate clustertriggerauthentication when RoleArn is not empty and err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) + roleArn := "Hello" identityOwner := workloadString - spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAws, "Helo", nil, &identityOwner) + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAws, &roleArn, nil, nil, nil, &identityOwner) ta := createTriggerAuthentication("clusteridentityidta", namespaceName, "ClusterTriggerAuthentication", spec) Eventually(func() error { return k8sClient.Create(context.Background(), ta) @@ -223,7 +306,7 @@ var _ = It("validate clustertriggerauthentication when RoleArn is empty and Iden Expect(err).ToNot(HaveOccurred()) identityOwner := kedaString - spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAws, "", nil, &identityOwner) + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAws, nil, nil, nil, nil, &identityOwner) ta := createTriggerAuthentication("clusteridentityidta", namespaceName, "ClusterTriggerAuthentication", spec) Eventually(func() error { return k8sClient.Create(context.Background(), ta) @@ -237,20 +320,22 @@ var _ = It("validate clustertriggerauthentication when RoleArn is not empty and Expect(err).ToNot(HaveOccurred()) identityOwner := workloadString - spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAws, "", nil, &identityOwner) + spec := createTriggerAuthenticationSpecWithPodIdentity(PodIdentityProviderAws, nil, nil, nil, nil, &identityOwner) ta := createTriggerAuthentication("clusteridentityidta", namespaceName, "TriggerAuthentication", spec) Eventually(func() error { return k8sClient.Create(context.Background(), ta) }).ShouldNot(HaveOccurred()) }) -func createTriggerAuthenticationSpecWithPodIdentity(provider PodIdentityProvider, roleArn string, identityID, identityOwner *string) TriggerAuthenticationSpec { +func createTriggerAuthenticationSpecWithPodIdentity(provider PodIdentityProvider, roleArn, identityID, identityTenantID, identityAuthorityHost, identityOwner *string) TriggerAuthenticationSpec { return TriggerAuthenticationSpec{ PodIdentity: &AuthPodIdentity{ - Provider: provider, - IdentityID: identityID, - RoleArn: roleArn, - IdentityOwner: identityOwner, + Provider: provider, + IdentityID: identityID, + IdentityTenantID: identityTenantID, + IdentityAuthorityHost: identityAuthorityHost, + RoleArn: roleArn, + IdentityOwner: identityOwner, }, } } diff --git a/apis/keda/v1alpha1/zz_generated.deepcopy.go b/apis/keda/v1alpha1/zz_generated.deepcopy.go index 46af93efa0e..a144aeb07d3 100755 --- a/apis/keda/v1alpha1/zz_generated.deepcopy.go +++ b/apis/keda/v1alpha1/zz_generated.deepcopy.go @@ -85,6 +85,21 @@ func (in *AuthPodIdentity) DeepCopyInto(out *AuthPodIdentity) { *out = new(string) **out = **in } + if in.IdentityTenantID != nil { + in, out := &in.IdentityTenantID, &out.IdentityTenantID + *out = new(string) + **out = **in + } + if in.IdentityAuthorityHost != nil { + in, out := &in.IdentityAuthorityHost, &out.IdentityAuthorityHost + *out = new(string) + **out = **in + } + if in.RoleArn != nil { + in, out := &in.RoleArn, &out.RoleArn + *out = new(string) + **out = **in + } if in.IdentityOwner != nil { in, out := &in.IdentityOwner, &out.IdentityOwner *out = new(string) diff --git a/cmd/webhooks/main.go b/cmd/webhooks/main.go index 0670081712f..4f03ba99b63 100644 --- a/cmd/webhooks/main.go +++ b/cmd/webhooks/main.go @@ -146,4 +146,8 @@ func setupWebhook(mgr manager.Manager) { setupLog.Error(err, "unable to create webhook", "webhook", "ClusterTriggerAuthentication") os.Exit(1) } + if err := (&kedav1alpha1.ScaledJob{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "ScaledJob") + os.Exit(1) + } } diff --git a/config/crd/bases/keda.sh_clustertriggerauthentications.yaml b/config/crd/bases/keda.sh_clustertriggerauthentications.yaml index 05c21c07497..9fc8c9be167 100644 --- a/config/crd/bases/keda.sh_clustertriggerauthentications.yaml +++ b/config/crd/bases/keda.sh_clustertriggerauthentications.yaml @@ -138,6 +138,11 @@ spec: AuthPodIdentity allows users to select the platform native identity mechanism properties: + identityAuthorityHost: + description: Set identityAuthorityHost to override the default + Azure authority host. If this is set, then the IdentityTenantID + must also be set + type: string identityId: type: string identityOwner: @@ -148,6 +153,11 @@ spec: - keda - workload type: string + identityTenantId: + description: Set identityTenantId to override the default + Azure tenant id. If this is set, then the IdentityID must + also be set + type: string provider: description: PodIdentityProvider contains the list of providers enum: @@ -157,6 +167,7 @@ spec: - aws - aws-eks - aws-kiam + - none type: string roleArn: description: RoleArn sets the AWS RoleArn to be used. Mutually @@ -237,6 +248,11 @@ spec: AuthPodIdentity allows users to select the platform native identity mechanism properties: + identityAuthorityHost: + description: Set identityAuthorityHost to override the default + Azure authority host. If this is set, then the IdentityTenantID + must also be set + type: string identityId: type: string identityOwner: @@ -247,6 +263,11 @@ spec: - keda - workload type: string + identityTenantId: + description: Set identityTenantId to override the default + Azure tenant id. If this is set, then the IdentityID must + also be set + type: string provider: description: PodIdentityProvider contains the list of providers enum: @@ -256,6 +277,7 @@ spec: - aws - aws-eks - aws-kiam + - none type: string roleArn: description: RoleArn sets the AWS RoleArn to be used. Mutually @@ -350,6 +372,11 @@ spec: AuthPodIdentity allows users to select the platform native identity mechanism properties: + identityAuthorityHost: + description: Set identityAuthorityHost to override the default + Azure authority host. If this is set, then the IdentityTenantID + must also be set + type: string identityId: type: string identityOwner: @@ -360,6 +387,11 @@ spec: - keda - workload type: string + identityTenantId: + description: Set identityTenantId to override the default + Azure tenant id. If this is set, then the IdentityID must + also be set + type: string provider: description: PodIdentityProvider contains the list of providers enum: @@ -369,6 +401,7 @@ spec: - aws - aws-eks - aws-kiam + - none type: string roleArn: description: RoleArn sets the AWS RoleArn to be used. Mutually @@ -466,6 +499,11 @@ spec: AuthPodIdentity allows users to select the platform native identity mechanism properties: + identityAuthorityHost: + description: Set identityAuthorityHost to override the default + Azure authority host. If this is set, then the IdentityTenantID + must also be set + type: string identityId: type: string identityOwner: @@ -476,6 +514,11 @@ spec: - keda - workload type: string + identityTenantId: + description: Set identityTenantId to override the default Azure + tenant id. If this is set, then the IdentityID must also be + set + type: string provider: description: PodIdentityProvider contains the list of providers enum: @@ -485,6 +528,7 @@ spec: - aws - aws-eks - aws-kiam + - none type: string roleArn: description: RoleArn sets the AWS RoleArn to be used. Mutually diff --git a/config/crd/bases/keda.sh_triggerauthentications.yaml b/config/crd/bases/keda.sh_triggerauthentications.yaml index 6bb8748c429..a6ee0563165 100644 --- a/config/crd/bases/keda.sh_triggerauthentications.yaml +++ b/config/crd/bases/keda.sh_triggerauthentications.yaml @@ -137,6 +137,11 @@ spec: AuthPodIdentity allows users to select the platform native identity mechanism properties: + identityAuthorityHost: + description: Set identityAuthorityHost to override the default + Azure authority host. If this is set, then the IdentityTenantID + must also be set + type: string identityId: type: string identityOwner: @@ -147,6 +152,11 @@ spec: - keda - workload type: string + identityTenantId: + description: Set identityTenantId to override the default + Azure tenant id. If this is set, then the IdentityID must + also be set + type: string provider: description: PodIdentityProvider contains the list of providers enum: @@ -156,6 +166,7 @@ spec: - aws - aws-eks - aws-kiam + - none type: string roleArn: description: RoleArn sets the AWS RoleArn to be used. Mutually @@ -236,6 +247,11 @@ spec: AuthPodIdentity allows users to select the platform native identity mechanism properties: + identityAuthorityHost: + description: Set identityAuthorityHost to override the default + Azure authority host. If this is set, then the IdentityTenantID + must also be set + type: string identityId: type: string identityOwner: @@ -246,6 +262,11 @@ spec: - keda - workload type: string + identityTenantId: + description: Set identityTenantId to override the default + Azure tenant id. If this is set, then the IdentityID must + also be set + type: string provider: description: PodIdentityProvider contains the list of providers enum: @@ -255,6 +276,7 @@ spec: - aws - aws-eks - aws-kiam + - none type: string roleArn: description: RoleArn sets the AWS RoleArn to be used. Mutually @@ -349,6 +371,11 @@ spec: AuthPodIdentity allows users to select the platform native identity mechanism properties: + identityAuthorityHost: + description: Set identityAuthorityHost to override the default + Azure authority host. If this is set, then the IdentityTenantID + must also be set + type: string identityId: type: string identityOwner: @@ -359,6 +386,11 @@ spec: - keda - workload type: string + identityTenantId: + description: Set identityTenantId to override the default + Azure tenant id. If this is set, then the IdentityID must + also be set + type: string provider: description: PodIdentityProvider contains the list of providers enum: @@ -368,6 +400,7 @@ spec: - aws - aws-eks - aws-kiam + - none type: string roleArn: description: RoleArn sets the AWS RoleArn to be used. Mutually @@ -465,6 +498,11 @@ spec: AuthPodIdentity allows users to select the platform native identity mechanism properties: + identityAuthorityHost: + description: Set identityAuthorityHost to override the default + Azure authority host. If this is set, then the IdentityTenantID + must also be set + type: string identityId: type: string identityOwner: @@ -475,6 +513,11 @@ spec: - keda - workload type: string + identityTenantId: + description: Set identityTenantId to override the default Azure + tenant id. If this is set, then the IdentityID must also be + set + type: string provider: description: PodIdentityProvider contains the list of providers enum: @@ -484,6 +527,7 @@ spec: - aws - aws-eks - aws-kiam + - none type: string roleArn: description: RoleArn sets the AWS RoleArn to be used. Mutually diff --git a/config/webhooks/validation_webhooks.yaml b/config/webhooks/validation_webhooks.yaml index 3561df56e22..0e35590bc11 100644 --- a/config/webhooks/validation_webhooks.yaml +++ b/config/webhooks/validation_webhooks.yaml @@ -33,6 +33,30 @@ webhooks: - scaledobjects sideEffects: None timeoutSeconds: 10 +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: keda-admission-webhooks + namespace: keda + path: /validate-keda-sh-v1alpha1-scaledjob + failurePolicy: Ignore + matchPolicy: Equivalent + name: vscaledjob.kb.io + namespaceSelector: {} + objectSelector: {} + rules: + - apiGroups: + - keda.sh + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - scaledjobs + sideEffects: None + timeoutSeconds: 10 - admissionReviewVersions: - v1 clientConfig: diff --git a/controllers/keda/scaledjob_controller.go b/controllers/keda/scaledjob_controller.go index 1adde787874..f381983e720 100755 --- a/controllers/keda/scaledjob_controller.go +++ b/controllers/keda/scaledjob_controller.go @@ -182,6 +182,11 @@ func (r *ScaledJobReconciler) reconcileScaledJob(ctx context.Context, logger log return "ScaledJob is paused, skipping reconcile loop", err } + err = kedav1alpha1.ValidateTriggers(scaledJob.Spec.Triggers) + if err != nil { + return "ScaledJob doesn't have correct triggers specification", err + } + // nosemgrep: trailofbits.go.invalid-usage-of-modified-variable.invalid-usage-of-modified-variable msg, err := r.deletePreviousVersionScaleJobs(ctx, logger, scaledJob) if err != nil { diff --git a/controllers/keda/scaledjob_controller_test.go b/controllers/keda/scaledjob_controller_test.go index eb60bbd3b2c..d4b95c712c9 100644 --- a/controllers/keda/scaledjob_controller_test.go +++ b/controllers/keda/scaledjob_controller_test.go @@ -152,6 +152,35 @@ var _ = Describe("ScaledJobController", func() { }).WithTimeout(1 * time.Minute).WithPolling(10 * time.Second).Should(Equal(metav1.ConditionUnknown)) }) + // Fix issue 5520 + It("create scaledjob with empty triggers should be blocked", func() { + // Create the ScaledJob without specifying name. + jobName := "empty-triggers-sj-name" + sjName := "sj-" + jobName + // create object already paused + sj := &kedav1alpha1.ScaledJob{ + ObjectMeta: metav1.ObjectMeta{ + Name: sjName, + Namespace: "default", + }, + Spec: kedav1alpha1.ScaledJobSpec{ + JobTargetRef: generateJobSpec(jobName), + Triggers: []kedav1alpha1.ScaleTriggers{}, + }, + } + + err := k8sClient.Create(context.Background(), sj) + Expect(err).ToNot(HaveOccurred()) + + // wait to check sj's ready condition Not Ready + Eventually(func() metav1.ConditionStatus { + err := k8sClient.Get(context.Background(), types.NamespacedName{Name: sjName, Namespace: "default"}, sj) + if err != nil { + return metav1.ConditionUnknown + } + return sj.Status.Conditions.GetReadyCondition().Status + }).Should(Equal(metav1.ConditionFalse)) + }) }) }) diff --git a/controllers/keda/scaledobject_controller_test.go b/controllers/keda/scaledobject_controller_test.go index af39a09acba..059025fc2ab 100644 --- a/controllers/keda/scaledobject_controller_test.go +++ b/controllers/keda/scaledobject_controller_test.go @@ -1396,6 +1396,51 @@ var _ = Describe("ScaledObjectController", func() { }).Should(BeNil()) }) + // Fix issue 5520 + It("create scaledobject with empty triggers should be blocked", func() { + var ( + deploymentName = "block-deleted" + soName = "so-" + deploymentName + min int32 = 1 + max int32 = 5 + pollingInterVal int32 = 1 + ) + + err := k8sClient.Create(context.Background(), generateDeployment(deploymentName)) + Expect(err).ToNot(HaveOccurred()) + + // Create the ScaledObject without specifying name. + so := &kedav1alpha1.ScaledObject{ + ObjectMeta: metav1.ObjectMeta{ + Name: soName, + Namespace: "default", + }, + Spec: kedav1alpha1.ScaledObjectSpec{ + ScaleTargetRef: &kedav1alpha1.ScaleTarget{ + Name: deploymentName, + }, + MinReplicaCount: &min, + MaxReplicaCount: &max, + PollingInterval: &pollingInterVal, + Advanced: &kedav1alpha1.AdvancedConfig{ + HorizontalPodAutoscalerConfig: &kedav1alpha1.HorizontalPodAutoscalerConfig{}, + }, + Triggers: []kedav1alpha1.ScaleTriggers{}, + }, + } + err = k8sClient.Create(context.Background(), so) + Expect(err).ToNot(HaveOccurred()) + + // wait to check so's ready condition Not Ready + Eventually(func() metav1.ConditionStatus { + err := k8sClient.Get(context.Background(), types.NamespacedName{Name: soName, Namespace: "default"}, so) + if err != nil { + return metav1.ConditionUnknown + } + return so.Status.Conditions.GetReadyCondition().Status + }).Should(Equal(metav1.ConditionFalse)) + }) + }) func generateDeployment(name string) *appsv1.Deployment { diff --git a/pkg/scalers/azure/azure_aad_workload_identity.go b/pkg/scalers/azure/azure_aad_workload_identity.go index e8d4ac89549..06be2f09c97 100644 --- a/pkg/scalers/azure/azure_aad_workload_identity.go +++ b/pkg/scalers/azure/azure_aad_workload_identity.go @@ -41,28 +41,40 @@ const ( azureClientIDEnv = "AZURE_CLIENT_ID" azureTenantIDEnv = "AZURE_TENANT_ID" azureFederatedTokenFileEnv = "AZURE_FEDERATED_TOKEN_FILE" - azureAuthrityHostEnv = "AZURE_AUTHORITY_HOST" + azureAuthorityHostEnv = "AZURE_AUTHORITY_HOST" ) var DefaultClientID string -var TenantID string +var DefaultTenantID string var TokenFilePath string -var AuthorityHost string +var DefaultAuthorityHost string func init() { DefaultClientID = os.Getenv(azureClientIDEnv) - TenantID = os.Getenv(azureTenantIDEnv) + DefaultTenantID = os.Getenv(azureTenantIDEnv) TokenFilePath = os.Getenv(azureFederatedTokenFileEnv) - AuthorityHost = os.Getenv(azureAuthrityHostEnv) + DefaultAuthorityHost = os.Getenv(azureAuthorityHostEnv) } // GetAzureADWorkloadIdentityToken returns the AADToken for resource -func GetAzureADWorkloadIdentityToken(ctx context.Context, identityID, resource string) (AADToken, error) { +func GetAzureADWorkloadIdentityToken(ctx context.Context, identityID, identityTenantID, identityAuthorityHost, resource string) (AADToken, error) { clientID := DefaultClientID + tenantID := DefaultTenantID + authorityHost := DefaultAuthorityHost + if identityID != "" { clientID = identityID } + if identityTenantID != "" { + tenantID = identityTenantID + + // override the authority host only if provided and tenant id is provided + if identityAuthorityHost != "" { + authorityHost = identityAuthorityHost + } + } + signedAssertion, err := readJWTFromFileSystem(TokenFilePath) if err != nil { return AADToken{}, fmt.Errorf("error reading service account token - %w", err) @@ -77,7 +89,7 @@ func GetAzureADWorkloadIdentityToken(ctx context.Context, identityID, resource s }) confidentialClient, err := confidential.New( - fmt.Sprintf("%s%s/oauth2/token", AuthorityHost, TenantID), + fmt.Sprintf("%s%s/oauth2/token", authorityHost, tenantID), clientID, cred, ) @@ -117,26 +129,31 @@ func getScopedResource(resource string) string { } type ADWorkloadIdentityConfig struct { - ctx context.Context - IdentityID string - Resource string + ctx context.Context + IdentityID string + IdentityTenantID string + IdentityAuthorityHost string + Resource string } -func NewAzureADWorkloadIdentityConfig(ctx context.Context, identityID, resource string) auth.AuthorizerConfig { - return ADWorkloadIdentityConfig{ctx: ctx, IdentityID: identityID, Resource: resource} +func NewAzureADWorkloadIdentityConfig(ctx context.Context, identityID, identityTenantID, identityAuthorityHost, resource string) auth.AuthorizerConfig { + return ADWorkloadIdentityConfig{ctx: ctx, IdentityID: identityID, IdentityTenantID: identityTenantID, IdentityAuthorityHost: identityAuthorityHost, Resource: resource} } // Authorizer implements the auth.AuthorizerConfig interface func (aadWiConfig ADWorkloadIdentityConfig) Authorizer() (autorest.Authorizer, error) { return autorest.NewBearerAuthorizer(NewAzureADWorkloadIdentityTokenProvider( - aadWiConfig.ctx, aadWiConfig.IdentityID, aadWiConfig.Resource)), nil + aadWiConfig.ctx, aadWiConfig.IdentityID, aadWiConfig.IdentityTenantID, aadWiConfig.IdentityAuthorityHost, aadWiConfig.Resource)), nil } -func NewADWorkloadIdentityCredential(identityID string) (*azidentity.WorkloadIdentityCredential, error) { +func NewADWorkloadIdentityCredential(identityID, identityTenantID string) (*azidentity.WorkloadIdentityCredential, error) { options := &azidentity.WorkloadIdentityCredentialOptions{} if identityID != "" { options.ClientID = identityID } + if identityTenantID != "" { + options.TenantID = identityTenantID + } return azidentity.NewWorkloadIdentityCredential(options) } @@ -144,14 +161,16 @@ func NewADWorkloadIdentityCredential(identityID string) (*azidentity.WorkloadIde // The OAuthTokenProvider interface is used by the BearerAuthorizer to get the token when preparing the HTTP Header. // The Refresher interface is used by the BearerAuthorizer to refresh the token. type ADWorkloadIdentityTokenProvider struct { - ctx context.Context - IdentityID string - Resource string - aadToken AADToken + ctx context.Context + IdentityID string + IdentityTenantID string + IdentityAuthorityHost string + Resource string + aadToken AADToken } -func NewAzureADWorkloadIdentityTokenProvider(ctx context.Context, identityID, resource string) *ADWorkloadIdentityTokenProvider { - return &ADWorkloadIdentityTokenProvider{ctx: ctx, IdentityID: identityID, Resource: resource} +func NewAzureADWorkloadIdentityTokenProvider(ctx context.Context, identityID, identityTenantID, identityAuthorityHost, resource string) *ADWorkloadIdentityTokenProvider { + return &ADWorkloadIdentityTokenProvider{ctx: ctx, IdentityID: identityID, IdentityTenantID: identityTenantID, IdentityAuthorityHost: identityAuthorityHost, Resource: resource} } // OAuthToken is for implementing the adal.OAuthTokenProvider interface. It returns the current access token. @@ -165,7 +184,7 @@ func (wiTokenProvider *ADWorkloadIdentityTokenProvider) Refresh() error { return nil } - aadToken, err := GetAzureADWorkloadIdentityToken(wiTokenProvider.ctx, wiTokenProvider.IdentityID, wiTokenProvider.Resource) + aadToken, err := GetAzureADWorkloadIdentityToken(wiTokenProvider.ctx, wiTokenProvider.IdentityID, wiTokenProvider.IdentityTenantID, wiTokenProvider.IdentityAuthorityHost, wiTokenProvider.Resource) if err != nil { return err } diff --git a/pkg/scalers/azure/azure_app_insights.go b/pkg/scalers/azure/azure_app_insights.go index 461d61a2cf9..0ce57b6d9eb 100644 --- a/pkg/scalers/azure/azure_app_insights.go +++ b/pkg/scalers/azure/azure_app_insights.go @@ -72,7 +72,7 @@ func getAuthConfig(ctx context.Context, info AppInsightsInfo, podIdentity kedav1 config.ClientID = podIdentity.GetIdentityID() return config case kedav1alpha1.PodIdentityProviderAzureWorkload: - return NewAzureADWorkloadIdentityConfig(ctx, podIdentity.GetIdentityID(), info.AppInsightsResourceURL) + return NewAzureADWorkloadIdentityConfig(ctx, podIdentity.GetIdentityID(), podIdentity.GetIdentityTenantID(), podIdentity.GetIdentityAuthorityHost(), info.AppInsightsResourceURL) } return nil } diff --git a/pkg/scalers/azure/azure_azidentity_chain.go b/pkg/scalers/azure/azure_azidentity_chain.go index d23841ab60a..0396c235a42 100644 --- a/pkg/scalers/azure/azure_azidentity_chain.go +++ b/pkg/scalers/azure/azure_azidentity_chain.go @@ -10,7 +10,7 @@ import ( "github.com/kedacore/keda/v2/apis/keda/v1alpha1" ) -func NewChainedCredential(logger logr.Logger, identityID string, podIdentity v1alpha1.PodIdentityProvider) (*azidentity.ChainedTokenCredential, error) { +func NewChainedCredential(logger logr.Logger, identityID, identityTenantID string, podIdentity v1alpha1.PodIdentityProvider) (*azidentity.ChainedTokenCredential, error) { var creds []azcore.TokenCredential // Used for local debug based on az-cli user @@ -42,7 +42,7 @@ func NewChainedCredential(logger logr.Logger, identityID string, podIdentity v1a creds = append(creds, msiCred) } case v1alpha1.PodIdentityProviderAzureWorkload: - wiCred, err := NewADWorkloadIdentityCredential(identityID) + wiCred, err := NewADWorkloadIdentityCredential(identityID, identityTenantID) if err != nil { logger.Error(err, "error starting azure workload-identity token provider") } else { diff --git a/pkg/scalers/azure/azure_data_explorer.go b/pkg/scalers/azure/azure_data_explorer.go index 7b187436372..db5e1295213 100644 --- a/pkg/scalers/azure/azure_data_explorer.go +++ b/pkg/scalers/azure/azure_data_explorer.go @@ -91,7 +91,7 @@ func getDataExplorerAuthConfig(metadata *DataExplorerMetadata) (*kusto.Connectio case kedav1alpha1.PodIdentityProviderAzure, kedav1alpha1.PodIdentityProviderAzureWorkload: azureDataExplorerLogger.V(1).Info(fmt.Sprintf("Creating Azure Data Explorer Client using podIdentity %s", metadata.PodIdentity.Provider)) - creds, chainedErr := NewChainedCredential(azureDataExplorerLogger, metadata.PodIdentity.GetIdentityID(), metadata.PodIdentity.Provider) + creds, chainedErr := NewChainedCredential(azureDataExplorerLogger, metadata.PodIdentity.GetIdentityID(), metadata.PodIdentity.GetIdentityTenantID(), metadata.PodIdentity.Provider) if chainedErr != nil { return nil, chainedErr } diff --git a/pkg/scalers/azure/azure_eventhub.go b/pkg/scalers/azure/azure_eventhub.go index d4ca34b879c..0ff4e347521 100644 --- a/pkg/scalers/azure/azure_eventhub.go +++ b/pkg/scalers/azure/azure_eventhub.go @@ -68,7 +68,7 @@ func GetEventHubClient(ctx context.Context, info EventHubInfo) (*eventhub.Hub, e // User wants to use AAD Workload Identity env := azure.Environment{ActiveDirectoryEndpoint: info.ActiveDirectoryEndpoint, ServiceBusEndpointSuffix: info.ServiceBusEndpointSuffix} hubEnvOptions := eventhub.HubWithEnvironment(env) - provider := NewAzureADWorkloadIdentityTokenProvider(ctx, info.PodIdentity.GetIdentityID(), info.EventHubResourceURL) + provider := NewAzureADWorkloadIdentityTokenProvider(ctx, info.PodIdentity.GetIdentityID(), info.PodIdentity.GetIdentityTenantID(), info.PodIdentity.GetIdentityAuthorityHost(), info.EventHubResourceURL) return eventhub.NewHub(info.Namespace, info.EventHubName, provider, hubEnvOptions) } diff --git a/pkg/scalers/azure/azure_managed_prometheus_http_round_tripper.go b/pkg/scalers/azure/azure_managed_prometheus_http_round_tripper.go index 33dc15dbb85..40cc65b9f2d 100644 --- a/pkg/scalers/azure/azure_managed_prometheus_http_round_tripper.go +++ b/pkg/scalers/azure/azure_managed_prometheus_http_round_tripper.go @@ -37,7 +37,7 @@ func TryAndGetAzureManagedPrometheusHTTPRoundTripper(logger logr.Logger, podIden return nil, fmt.Errorf("trigger metadata cannot be nil") } - chainedCred, err := NewChainedCredential(logger, podIdentity.GetIdentityID(), podIdentity.Provider) + chainedCred, err := NewChainedCredential(logger, podIdentity.GetIdentityID(), podIdentity.GetIdentityTenantID(), podIdentity.Provider) if err != nil { return nil, err } diff --git a/pkg/scalers/azure/azure_monitor.go b/pkg/scalers/azure/azure_monitor.go index 0ed25ff561c..d7bfe2281e6 100644 --- a/pkg/scalers/azure/azure_monitor.go +++ b/pkg/scalers/azure/azure_monitor.go @@ -93,7 +93,7 @@ func createMetricsClient(ctx context.Context, info MonitorInfo, podIdentity keda authConfig = config case kedav1alpha1.PodIdentityProviderAzureWorkload: - authConfig = NewAzureADWorkloadIdentityConfig(ctx, podIdentity.GetIdentityID(), info.AzureResourceManagerEndpoint) + authConfig = NewAzureADWorkloadIdentityConfig(ctx, podIdentity.GetIdentityID(), podIdentity.GetIdentityTenantID(), podIdentity.GetIdentityAuthorityHost(), info.AzureResourceManagerEndpoint) } authorizer, _ := authConfig.Authorizer() diff --git a/pkg/scalers/azure/azure_storage.go b/pkg/scalers/azure/azure_storage.go index d8f09d2f2ee..0c63e065f17 100644 --- a/pkg/scalers/azure/azure_storage.go +++ b/pkg/scalers/azure/azure_storage.go @@ -227,7 +227,7 @@ func parseAccessTokenAndEndpoint(ctx context.Context, httpClient util.HTTPDoer, case kedav1alpha1.PodIdentityProviderAzure: token, err = GetAzureADPodIdentityToken(ctx, httpClient, podIdentity.GetIdentityID(), storageResource) case kedav1alpha1.PodIdentityProviderAzureWorkload: - token, err = GetAzureADWorkloadIdentityToken(ctx, podIdentity.GetIdentityID(), storageResource) + token, err = GetAzureADWorkloadIdentityToken(ctx, podIdentity.GetIdentityID(), podIdentity.GetIdentityTenantID(), podIdentity.GetIdentityAuthorityHost(), storageResource) } if err != nil { diff --git a/pkg/scalers/azure_log_analytics_scaler.go b/pkg/scalers/azure_log_analytics_scaler.go index b07abdacf57..7cae83735c9 100644 --- a/pkg/scalers/azure_log_analytics_scaler.go +++ b/pkg/scalers/azure_log_analytics_scaler.go @@ -477,7 +477,7 @@ func (s *azureLogAnalyticsScaler) getAuthorizationToken(ctx context.Context) (to switch s.metadata.podIdentity.Provider { case kedav1alpha1.PodIdentityProviderAzureWorkload: - aadToken, err := azure.GetAzureADWorkloadIdentityToken(ctx, s.metadata.podIdentity.GetIdentityID(), s.metadata.logAnalyticsResourceURL) + aadToken, err := azure.GetAzureADWorkloadIdentityToken(ctx, s.metadata.podIdentity.GetIdentityID(), s.metadata.podIdentity.GetIdentityTenantID(), s.metadata.podIdentity.GetIdentityAuthorityHost(), s.metadata.logAnalyticsResourceURL) if err != nil { return tokenData{}, nil } diff --git a/pkg/scalers/azure_pipelines_scaler.go b/pkg/scalers/azure_pipelines_scaler.go index 8413642e68e..6bdf5c3fdec 100644 --- a/pkg/scalers/azure_pipelines_scaler.go +++ b/pkg/scalers/azure_pipelines_scaler.go @@ -194,7 +194,7 @@ func getAuthMethod(logger logr.Logger, config *scalersconfig.ScalerConfig) (stri case "", kedav1alpha1.PodIdentityProviderNone: return "", nil, kedav1alpha1.AuthPodIdentity{}, fmt.Errorf("no personalAccessToken given or PodIdentity provider configured") case kedav1alpha1.PodIdentityProviderAzure, kedav1alpha1.PodIdentityProviderAzureWorkload: - cred, err := azure.NewChainedCredential(logger, config.PodIdentity.GetIdentityID(), config.PodIdentity.Provider) + cred, err := azure.NewChainedCredential(logger, config.PodIdentity.GetIdentityID(), config.PodIdentity.GetIdentityTenantID(), config.PodIdentity.Provider) if err != nil { return "", nil, kedav1alpha1.AuthPodIdentity{}, err } diff --git a/pkg/scalers/azure_servicebus_scaler.go b/pkg/scalers/azure_servicebus_scaler.go index b3ee21614c2..18973b71459 100755 --- a/pkg/scalers/azure_servicebus_scaler.go +++ b/pkg/scalers/azure_servicebus_scaler.go @@ -298,7 +298,7 @@ func (s *azureServiceBusScaler) getServiceBusAdminClient() (*admin.Client, error case "", kedav1alpha1.PodIdentityProviderNone: client, err = admin.NewClientFromConnectionString(s.metadata.connection, nil) case kedav1alpha1.PodIdentityProviderAzure, kedav1alpha1.PodIdentityProviderAzureWorkload: - creds, chainedErr := azure.NewChainedCredential(s.logger, s.podIdentity.GetIdentityID(), s.podIdentity.Provider) + creds, chainedErr := azure.NewChainedCredential(s.logger, s.podIdentity.GetIdentityID(), s.podIdentity.GetIdentityTenantID(), s.podIdentity.Provider) if chainedErr != nil { return nil, chainedErr } diff --git a/pkg/scalers/kafka_scaler.go b/pkg/scalers/kafka_scaler.go index cae219bc533..86acc802d81 100644 --- a/pkg/scalers/kafka_scaler.go +++ b/pkg/scalers/kafka_scaler.go @@ -75,9 +75,10 @@ type kafkaMetadata struct { password string // GSSAPI - keytabPath string - realm string - kerberosConfigPath string + keytabPath string + realm string + kerberosConfigPath string + kerberosServiceName string // OAUTHBEARER scopes []string @@ -291,6 +292,10 @@ func parseKerberosParams(config *scalersconfig.ScalerConfig, meta *kafkaMetadata } meta.kerberosConfigPath = path + if config.AuthParams["kerberosServiceName"] != "" { + meta.kerberosServiceName = strings.TrimSpace(config.AuthParams["kerberosServiceName"]) + } + meta.saslType = mode return nil } @@ -541,7 +546,11 @@ func getKafkaClients(metadata kafkaMetadata) (sarama.Client, sarama.ClusterAdmin if metadata.saslType == KafkaSASLTypeGSSAPI { config.Net.SASL.Enable = true config.Net.SASL.Mechanism = sarama.SASLTypeGSSAPI - config.Net.SASL.GSSAPI.ServiceName = "kafka" + if metadata.kerberosServiceName != "" { + config.Net.SASL.GSSAPI.ServiceName = metadata.kerberosServiceName + } else { + config.Net.SASL.GSSAPI.ServiceName = "kafka" + } config.Net.SASL.GSSAPI.Username = metadata.username config.Net.SASL.GSSAPI.Realm = metadata.realm config.Net.SASL.GSSAPI.KerberosConfigPath = metadata.kerberosConfigPath diff --git a/pkg/scalers/kafka_scaler_test.go b/pkg/scalers/kafka_scaler_test.go index eb679212b7a..81ebc746443 100644 --- a/pkg/scalers/kafka_scaler_test.go +++ b/pkg/scalers/kafka_scaler_test.go @@ -161,6 +161,8 @@ var parseKafkaAuthParamsTestDataset = []parseKafkaAuthParamsTestData{ {map[string]string{"sasl": "gssapi", "username": "admin", "password": "admin", "kerberosConfig": "", "realm": "tst.com", "tls": "enable", "ca": "caaa", "cert": "ceert", "key": "keey"}, false, true}, // success, SASL GSSAPI/keytab + TLS {map[string]string{"sasl": "gssapi", "username": "admin", "keytab": "/path/to/keytab", "kerberosConfig": "", "realm": "tst.com", "tls": "enable", "ca": "caaa", "cert": "ceert", "key": "keey"}, false, true}, + // success, SASL GSSAPI, KerberosServiceName supported + {map[string]string{"sasl": "gssapi", "username": "admin", "keytab": "/path/to/keytab", "kerberosConfig": "", "realm": "tst.com", "kerberosServiceName": "srckafka"}, false, false}, // failure, SASL OAUTHBEARER + TLS bad sasl type {map[string]string{"sasl": "foo", "username": "admin", "password": "admin", "scopes": "scope", "oauthTokenEndpointUri": "https://website.com", "tls": "disable"}, true, false}, // success, SASL OAUTHBEARER + TLS missing scope @@ -412,6 +414,9 @@ func TestKafkaAuthParamsInTriggerAuthentication(t *testing.T) { t.Errorf(err.Error()) } } + if meta.kerberosServiceName != testData.authParams["kerberosServiceName"] { + t.Errorf("Expected kerberos ServiceName to be set to %v but got %v\n", testData.authParams["kerberosServiceName"], meta.kerberosServiceName) + } } } } diff --git a/pkg/scalers/mongo_scaler.go b/pkg/scalers/mongo_scaler.go index 8e80b723baf..f7871c8567e 100644 --- a/pkg/scalers/mongo_scaler.go +++ b/pkg/scalers/mongo_scaler.go @@ -34,6 +34,9 @@ type mongoDBMetadata struct { // The string is used by connected with mongoDB. // +optional connectionString string + // Specify the prefix to connect to the mongoDB server, default value `mongodb`, if the connectionString be provided, don't need to specify this param. + // +optional + scheme string // Specify the host to connect to the mongoDB server,if the connectionString be provided, don't need to specify this param. // +optional host string @@ -162,6 +165,13 @@ func parseMongoDBMetadata(config *scalersconfig.ScalerConfig) (*mongoDBMetadata, meta.connectionString = config.ResolvedEnv[config.TriggerMetadata["connectionStringFromEnv"]] default: meta.connectionString = "" + scheme, err := GetFromAuthOrMeta(config, "scheme") + if err != nil { + meta.scheme = "mongodb" + } else { + meta.scheme = scheme + } + host, err := GetFromAuthOrMeta(config, "host") if err != nil { return nil, "", err @@ -196,7 +206,7 @@ func parseMongoDBMetadata(config *scalersconfig.ScalerConfig) (*mongoDBMetadata, // Build connection str addr := net.JoinHostPort(meta.host, meta.port) // nosemgrep: db-connection-string - connStr = fmt.Sprintf("mongodb://%s:%s@%s/%s", url.QueryEscape(meta.username), url.QueryEscape(meta.password), addr, meta.dbName) + connStr = fmt.Sprintf("%s://%s:%s@%s/%s", meta.scheme, url.QueryEscape(meta.username), url.QueryEscape(meta.password), addr, meta.dbName) } meta.triggerIndex = config.TriggerIndex return &meta, connStr, nil diff --git a/pkg/scalers/mongo_scaler_test.go b/pkg/scalers/mongo_scaler_test.go index 4ccb02b30f8..02f1e9479ef 100644 --- a/pkg/scalers/mongo_scaler_test.go +++ b/pkg/scalers/mongo_scaler_test.go @@ -70,6 +70,13 @@ var testMONGODBMetadata = []parseMongoDBMetadataTestData{ resolvedEnv: testMongoDBResolvedEnv, raisesError: false, }, + // mongodb srv support + { + metadata: map[string]string{"query": `{"name":"John"}`, "collection": "demo", "queryValue": "12"}, + authParams: map[string]string{"dbName": "test", "scheme": "mongodb+srv", "host": "localhost", "port": "1234", "username": "sample", "password": "sec@ure"}, + resolvedEnv: testMongoDBResolvedEnv, + raisesError: false, + }, // wrong activationQueryValue { metadata: map[string]string{"query": `{"name":"John"}`, "collection": "demo", "queryValue": "12", "activationQueryValue": "aa", "connectionStringFromEnv": "Mongo_CONN_STR", "dbName": "test"}, @@ -83,6 +90,7 @@ var mongoDBConnectionStringTestDatas = []mongoDBConnectionStringTestData{ {metadataTestData: &testMONGODBMetadata[2], connectionString: "mongodb://mongodb0.example.com:27017"}, {metadataTestData: &testMONGODBMetadata[3], connectionString: "mongodb://sample:test%40password@localhost:1234/test"}, {metadataTestData: &testMONGODBMetadata[4], connectionString: "mongodb://sample:sec%40ure@localhost:1234/test"}, + {metadataTestData: &testMONGODBMetadata[5], connectionString: "mongodb+srv://sample:sec%40ure@localhost:1234/test"}, } var mongoDBMetricIdentifiers = []mongoDBMetricIdentifier{ diff --git a/pkg/scalers/rabbitmq_scaler.go b/pkg/scalers/rabbitmq_scaler.go index 9681ff8f932..4e8cf3c2b5e 100644 --- a/pkg/scalers/rabbitmq_scaler.go +++ b/pkg/scalers/rabbitmq_scaler.go @@ -91,8 +91,10 @@ type rabbitMQMetadata struct { unsafeSsl bool // token provider for azure AD - workloadIdentityClientID string - workloadIdentityResource string + workloadIdentityClientID string + workloadIdentityTenantID string + workloadIdentityAuthorityHost string + workloadIdentityResource string } type queueInfo struct { @@ -244,6 +246,7 @@ func parseRabbitMQMetadata(config *scalersconfig.ScalerConfig) (*rabbitMQMetadat if config.PodIdentity.Provider == v1alpha1.PodIdentityProviderAzureWorkload { if config.AuthParams["workloadIdentityResource"] != "" { meta.workloadIdentityClientID = config.PodIdentity.GetIdentityID() + meta.workloadIdentityTenantID = config.PodIdentity.GetIdentityTenantID() meta.workloadIdentityResource = config.AuthParams["workloadIdentityResource"] } } @@ -510,7 +513,7 @@ func getJSON(ctx context.Context, s *rabbitMQScaler, url string) (queueInfo, err if s.metadata.workloadIdentityResource != "" { if s.azureOAuth == nil { - s.azureOAuth = azure.NewAzureADWorkloadIdentityTokenProvider(ctx, s.metadata.workloadIdentityClientID, s.metadata.workloadIdentityResource) + s.azureOAuth = azure.NewAzureADWorkloadIdentityTokenProvider(ctx, s.metadata.workloadIdentityClientID, s.metadata.workloadIdentityTenantID, s.metadata.workloadIdentityAuthorityHost, s.metadata.workloadIdentityResource) } err = s.azureOAuth.Refresh() diff --git a/pkg/scaling/resolver/aws_secretmanager_handler.go b/pkg/scaling/resolver/aws_secretmanager_handler.go index a85e0974601..3d10ba2e1e5 100644 --- a/pkg/scaling/resolver/aws_secretmanager_handler.go +++ b/pkg/scaling/resolver/aws_secretmanager_handler.go @@ -92,8 +92,8 @@ func (ash *AwsSecretManagerHandler) Initialize(ctx context.Context, client clien return fmt.Errorf("error resolving role arn for aws: %w", err) } ash.awsMetadata.AwsRoleArn = awsRoleArn - } else if ash.secretManager.PodIdentity.RoleArn != "" { - ash.awsMetadata.AwsRoleArn = ash.secretManager.PodIdentity.RoleArn + } else if ash.secretManager.PodIdentity.RoleArn != nil { + ash.awsMetadata.AwsRoleArn = *ash.secretManager.PodIdentity.RoleArn } default: return fmt.Errorf("pod identity provider %s not supported", podIdentity.Provider) diff --git a/pkg/scaling/resolver/azure_keyvault_handler.go b/pkg/scaling/resolver/azure_keyvault_handler.go index f7fe36544c2..3abd02f7ba6 100644 --- a/pkg/scaling/resolver/azure_keyvault_handler.go +++ b/pkg/scaling/resolver/azure_keyvault_handler.go @@ -137,7 +137,7 @@ func (vh *AzureKeyVaultHandler) getAuthConfig(ctx context.Context, client client return config, nil case kedav1alpha1.PodIdentityProviderAzureWorkload: - return azure.NewAzureADWorkloadIdentityConfig(ctx, podIdentity.GetIdentityID(), keyVaultResourceURL), nil + return azure.NewAzureADWorkloadIdentityConfig(ctx, podIdentity.GetIdentityID(), podIdentity.GetIdentityTenantID(), podIdentity.GetIdentityAuthorityHost(), keyVaultResourceURL), nil default: return nil, fmt.Errorf("key vault does not support pod identity provider - %s", podIdentity.Provider) } diff --git a/pkg/scaling/resolver/scale_resolvers.go b/pkg/scaling/resolver/scale_resolvers.go index 23c4ff8195c..3aa4ae28a29 100644 --- a/pkg/scaling/resolver/scale_resolvers.go +++ b/pkg/scaling/resolver/scale_resolvers.go @@ -187,12 +187,12 @@ func ResolveAuthRefAndPodIdentity(ctx context.Context, client client.Client, log } switch podIdentity.Provider { case kedav1alpha1.PodIdentityProviderAws: - if podIdentity.RoleArn != "" { + if podIdentity.RoleArn != nil { if podIdentity.IsWorkloadIdentityOwner() { return nil, kedav1alpha1.AuthPodIdentity{Provider: kedav1alpha1.PodIdentityProviderNone}, fmt.Errorf("roleArn can't be set if KEDA isn't identity owner, current value: '%s'", *podIdentity.IdentityOwner) } - authParams["awsRoleArn"] = podIdentity.RoleArn + authParams["awsRoleArn"] = *podIdentity.RoleArn } if podIdentity.IsWorkloadIdentityOwner() { value, err := resolveServiceAccountAnnotation(ctx, client, podTemplateSpec.Spec.ServiceAccountName, namespace, kedav1alpha1.PodIdentityAnnotationEKS, true) diff --git a/tests/helper/helper.go b/tests/helper/helper.go index f01d493cdd7..bbb3b8124b1 100644 --- a/tests/helper/helper.go +++ b/tests/helper/helper.go @@ -83,6 +83,8 @@ var ( GcpIdentityTests = os.Getenv("GCP_RUN_IDENTITY_TESTS") EnableOpentelemetry = os.Getenv("ENABLE_OPENTELEMETRY") InstallCertManager = AwsIdentityTests == StringTrue || GcpIdentityTests == StringTrue + InstallKeda = os.Getenv("E2E_INSTALL_KEDA") + InstallKafka = os.Getenv("E2E_INSTALL_KAFKA") ) var ( diff --git a/tests/internals/scaled_job_validation/scaled_job_validation_test.go b/tests/internals/scaled_job_validation/scaled_job_validation_test.go new file mode 100644 index 00000000000..62a2680e30f --- /dev/null +++ b/tests/internals/scaled_job_validation/scaled_job_validation_test.go @@ -0,0 +1,79 @@ +//go:build e2e +// +build e2e + +package cache_metrics_test + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + + . "github.com/kedacore/keda/v2/tests/helper" +) + +const ( + testName = "scaled-object-validation-test" +) + +var ( + testNamespace = fmt.Sprintf("%s-ns", testName) + emptyTriggersSjName = fmt.Sprintf("%s-sj-empty-triggers", testName) +) + +type templateData struct { + TestNamespace string + EmptyTriggersSjName string +} + +const ( + emptyTriggersSjTemplate = ` +apiVersion: keda.sh/v1alpha1 +kind: ScaledJob +metadata: + name: {{.EmptyTriggersSjName}} +spec: + jobTargetRef: + template: + spec: + containers: + - name: demo-rabbitmq-client + image: demo-rabbitmq-client:1 + imagePullPolicy: Always + command: ["receive", "amqp://user:PASSWORD@rabbitmq.default.svc.cluster.local:5672"] + envFrom: + - secretRef: + name: rabbitmq-consumer-secrets + restartPolicy: Never + triggers: [] +` +) + +func TestScaledJobValidations(t *testing.T) { + // setup + t.Log("--- setting up ---") + // Create kubernetes resources + kc := GetKubernetesClient(t) + data, templates := getTemplateData() + + CreateKubernetesResources(t, kc, testNamespace, data, templates) + + testTriggersWithEmptyArray(t, data) + + DeleteKubernetesResources(t, testNamespace, data, templates) +} + +func testTriggersWithEmptyArray(t *testing.T, data templateData) { + t.Log("--- triggers with empty array ---") + + err := KubectlApplyWithErrors(t, data, "emptyTriggersSjTemplate", emptyTriggersSjTemplate) + assert.Errorf(t, err, "can deploy the scaledJob - %s", err) + assert.Contains(t, err.Error(), "no triggers defined in the ScaledObject/ScaledJob") +} + +func getTemplateData() (templateData, []Template) { + return templateData{ + TestNamespace: testNamespace, + EmptyTriggersSjName: emptyTriggersSjName, + }, []Template{} +} diff --git a/tests/internals/scaled_object_validation/scaled_object_validation_test.go b/tests/internals/scaled_object_validation/scaled_object_validation_test.go index ca1bfbdc944..9cdaff34515 100644 --- a/tests/internals/scaled_object_validation/scaled_object_validation_test.go +++ b/tests/internals/scaled_object_validation/scaled_object_validation_test.go @@ -21,16 +21,18 @@ var ( deploymentName = fmt.Sprintf("%s-deployment", testName) scaledObject1Name = fmt.Sprintf("%s-so1", testName) scaledObject2Name = fmt.Sprintf("%s-so2", testName) + emptyTriggersSoName = fmt.Sprintf("%s-so-empty-triggers", testName) hpaName = fmt.Sprintf("%s-hpa", testName) ownershipTransferScaledObjectName = fmt.Sprintf("%s-ownership-transfer-so", testName) ownershipTransferHpaName = fmt.Sprintf("%s-ownership-transfer-hpa", testName) ) type templateData struct { - TestNamespace string - DeploymentName string - ScaledObjectName string - HpaName string + TestNamespace string + DeploymentName string + ScaledObjectName string + HpaName string + EmptyTriggersSoName string } const ( @@ -150,6 +152,18 @@ spec: type: Utilization averageUtilization: 50 ` + + emptyTriggersTemplate = ` +apiVersion: keda.sh/v1alpha1 +kind: ScaledObject +metadata: + name: {{.EmptyTriggersSoName}} + namespace: {{.TestNamespace}} +spec: + scaleTargetRef: + name: {{.DeploymentName}} + triggers: [] +` ) func TestScaledObjectValidations(t *testing.T) { @@ -175,6 +189,8 @@ func TestScaledObjectValidations(t *testing.T) { testWorkloadWithOnlyLimits(t, data) + testTriggersWithEmptyArray(t, data) + DeleteKubernetesResources(t, testNamespace, data, templates) } @@ -231,6 +247,7 @@ func testScaledWorkloadByOtherHpaWithOwnershipTransfer(t *testing.T, data templa assert.NoErrorf(t, err, "can deploy the scaledObject - %s", err) KubectlDeleteWithTemplate(t, data, "hpaTemplate", hpaTemplate) + KubectlDeleteWithTemplate(t, data, "ownershipTransferScaledObjectTemplate", ownershipTransferScaledObjectTemplate) } func testMissingCPU(t *testing.T, data templateData) { @@ -311,10 +328,19 @@ spec: assert.NoError(t, err, "Deployment with only resource limits set should be validated") } +func testTriggersWithEmptyArray(t *testing.T, data templateData) { + t.Log("--- triggers with empty array ---") + + err := KubectlApplyWithErrors(t, data, "emptyTriggersTemplate", emptyTriggersTemplate) + assert.Errorf(t, err, "can deploy the scaledObject - %s", err) + assert.Contains(t, err.Error(), "no triggers defined in the ScaledObject/ScaledJob") +} + func getTemplateData() (templateData, []Template) { return templateData{ - TestNamespace: testNamespace, - DeploymentName: deploymentName, + TestNamespace: testNamespace, + DeploymentName: deploymentName, + EmptyTriggersSoName: emptyTriggersSoName, }, []Template{ {Name: "deploymentTemplate", Config: deploymentTemplate}, } diff --git a/tests/utils/cleanup_test.go b/tests/utils/cleanup_test.go index f0d8b0a627b..049ca4fb7ba 100644 --- a/tests/utils/cleanup_test.go +++ b/tests/utils/cleanup_test.go @@ -14,6 +14,10 @@ import ( ) func TestRemoveKEDA(t *testing.T) { + // default to true + if InstallKeda == StringFalse { + t.Skip("skipping as requested -- KEDA not installed via these tests") + } out, err := ExecuteCommandWithDir("make undeploy", "../..") require.NoErrorf(t, err, "error removing KEDA - %s", err) @@ -65,6 +69,10 @@ func TestRemoveGcpIdentityComponents(t *testing.T) { } func TestRemoveOpentelemetryComponents(t *testing.T) { + if EnableOpentelemetry == "" || EnableOpentelemetry == StringFalse { + t.Skip("skipping uninstall of opentelemetry") + } + _, err := ExecuteCommand(fmt.Sprintf("helm uninstall opentelemetry-collector --namespace %s", OpentelemetryNamespace)) require.NoErrorf(t, err, "cannot uninstall opentelemetry-collector - %s", err) DeleteNamespace(t, OpentelemetryNamespace) @@ -85,6 +93,10 @@ func TestRemoveAzureManagedPrometheusComponents(t *testing.T) { } func TestRemoveStrimzi(t *testing.T) { + // default to true + if InstallKafka == StringFalse { + t.Skip("skipping as requested -- Kafka not managed by E2E tests") + } _, err := ExecuteCommand(fmt.Sprintf(`helm uninstall --namespace %s %s`, StrimziNamespace, StrimziChartName)) diff --git a/tests/utils/setup_test.go b/tests/utils/setup_test.go index 14cfcec67d7..f2d9e19eb84 100644 --- a/tests/utils/setup_test.go +++ b/tests/utils/setup_test.go @@ -204,6 +204,10 @@ func TestSetupOpentelemetryComponents(t *testing.T) { } func TestDeployKEDA(t *testing.T) { + // default to true + if InstallKeda == StringFalse { + t.Skip("skipping as requested -- KEDA assumed to be already installed") + } KubeClient = GetKubernetesClient(t) CreateNamespace(t, KubeClient, KEDANamespace) @@ -229,6 +233,10 @@ func TestDeployKEDA(t *testing.T) { } func TestVerifyKEDA(t *testing.T) { + // default to true + if InstallKeda == StringFalse { + t.Skip("skipping as requested -- KEDA assumed to be already installed") + } assert.True(t, WaitForDeploymentReplicaReadyCount(t, KubeClient, KEDAOperator, KEDANamespace, 1, 30, 6), "replica count should be 1 after 3 minutes") assert.True(t, WaitForDeploymentReplicaReadyCount(t, KubeClient, KEDAMetricsAPIServer, KEDANamespace, 1, 30, 6), @@ -268,6 +276,10 @@ func TestSetupAadPodIdentityComponents(t *testing.T) { } func TestSetUpStrimzi(t *testing.T) { + // default to true + if InstallKafka == StringFalse { + t.Skip("skipping as requested -- Kafka assumed to be unneeded or already installed") + } t.Log("--- installing kafka operator ---") _, err := ExecuteCommand("helm repo add strimzi https://strimzi.io/charts/") assert.NoErrorf(t, err, "cannot execute command - %s", err)