feat(operator): introduce fallback search to KLT default namespace wh…

…en KeptnTaskDefinition is not found (#1340)

Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>

operator/controllers/common/helperfunctions.go

@@ -1,16 +1,21 @@
 package common
 
 import (
+	"context"
 	"fmt"
 
+	"github.com/go-logr/logr"
 	klcv1alpha3 "github.com/keptn/lifecycle-toolkit/operator/apis/lifecycle/v1alpha3"
 	apicommon "github.com/keptn/lifecycle-toolkit/operator/apis/lifecycle/v1alpha3/common"
 	"github.com/keptn/lifecycle-toolkit/operator/controllers/lifecycle/interfaces"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+const KLTNamespace = "keptn-lifecycle-toolkit-system"
+
 // GetItemStatus retrieves the state of the task/evaluation, if it does not exists, it creates a default one
 func GetItemStatus(name string, instanceStatus []klcv1alpha3.ItemStatus) klcv1alpha3.ItemStatus {
 	for _, status := range instanceStatus {
@@ -81,3 +86,20 @@ func copyMap[M1 ~map[K]V, M2 ~map[K]V, K comparable, V any](dst M1, src M2) {
 		dst[k] = v
 	}
 }
+
+func GetTaskDefinition(k8sclient client.Client, log logr.Logger, ctx context.Context, definitionName string, namespace string) (*klcv1alpha3.KeptnTaskDefinition, error) {
+	definition := &klcv1alpha3.KeptnTaskDefinition{}
+	err := k8sclient.Get(ctx, types.NamespacedName{Name: definitionName, Namespace: namespace}, definition)
+	if err != nil {
+		log.Error(err, "Failed to get KeptnTaskDefinition from application namespace")
+		if k8serrors.IsNotFound(err) {
+			if err := k8sclient.Get(ctx, types.NamespacedName{Name: definitionName, Namespace: KLTNamespace}, definition); err != nil {
+				log.Error(err, "Failed to get KeptnTaskDefinition from default KLT namespace")
+				return nil, err
+			}
+			return definition, nil
+		}
+		return nil, err
+	}
+	return definition, nil
+}

operator/controllers/common/helperfunctions_test.go

@@ -1,14 +1,18 @@
 package common
 
 import (
+	"context"
 	"testing"
 
 	klcv1alpha3 "github.com/keptn/lifecycle-toolkit/operator/apis/lifecycle/v1alpha3"
 	apicommon "github.com/keptn/lifecycle-toolkit/operator/apis/lifecycle/v1alpha3/common"
 	"github.com/stretchr/testify/require"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/kubernetes/scheme"
+	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
 func Test_GetItemStatus(t *testing.T) {
@@ -402,3 +406,84 @@ func Test_setAnnotations(t *testing.T) {
 		})
 	}
 }
+
+func Test_GetTaskDefinition(t *testing.T) {
+	tests := []struct {
+		name             string
+		taskDef          *klcv1alpha3.KeptnTaskDefinition
+		taskDefName      string
+		taskDefNamespace string
+		out              *klcv1alpha3.KeptnTaskDefinition
+		wantError        bool
+	}{
+		{
+			name: "taskDef not found",
+			taskDef: &klcv1alpha3.KeptnTaskDefinition{
+				ObjectMeta: v1.ObjectMeta{
+					Name:      "taskDef",
+					Namespace: "some-other-namespace",
+				},
+			},
+			taskDefName:      "taskDef",
+			taskDefNamespace: "some-namespace",
+			out:              nil,
+			wantError:        true,
+		},
+		{
+			name: "taskDef found",
+			taskDef: &klcv1alpha3.KeptnTaskDefinition{
+				ObjectMeta: v1.ObjectMeta{
+					Name:      "taskDef",
+					Namespace: "some-namespace",
+				},
+			},
+			taskDefName:      "taskDef",
+			taskDefNamespace: "some-namespace",
+			out: &klcv1alpha3.KeptnTaskDefinition{
+				ObjectMeta: v1.ObjectMeta{
+					Name:      "taskDef",
+					Namespace: "some-namespace",
+				},
+			},
+			wantError: false,
+		},
+		{
+			name: "taskDef found in default KLT namespace",
+			taskDef: &klcv1alpha3.KeptnTaskDefinition{
+				ObjectMeta: v1.ObjectMeta{
+					Name:      "taskDef",
+					Namespace: KLTNamespace,
+				},
+			},
+			taskDefName:      "taskDef",
+			taskDefNamespace: "some-namespace",
+			out: &klcv1alpha3.KeptnTaskDefinition{
+				ObjectMeta: v1.ObjectMeta{
+					Name:      "taskDef",
+					Namespace: KLTNamespace,
+				},
+			},
+			wantError: false,
+		},
+	}
+
+	err := klcv1alpha3.AddToScheme(scheme.Scheme)
+	require.Nil(t, err)
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			client := fake.NewClientBuilder().WithObjects(tt.taskDef).Build()
+			d, err := GetTaskDefinition(client, ctrl.Log.WithName("testytest"), context.TODO(), tt.taskDefName, tt.taskDefNamespace)
+			if tt.out != nil && d != nil {
+				require.Equal(t, tt.out.Name, d.Name)
+				require.Equal(t, tt.out.Namespace, d.Namespace)
+			} else if tt.out != d {
+				t.Errorf("want: %v, got: %v", tt.out, d)
+			}
+			if tt.wantError != (err != nil) {
+				t.Errorf("want error: %t, got: %v", tt.wantError, err)
+			}
+
+		})
+	}
+}

operator/controllers/common/providers/keptnmetric/keptnmetric.go

@@ -7,6 +7,7 @@ import (
 	"github.com/go-logr/logr"
 	metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1alpha2"
 	klcv1alpha3 "github.com/keptn/lifecycle-toolkit/operator/apis/lifecycle/v1alpha3"
+	"github.com/keptn/lifecycle-toolkit/operator/controllers/common"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -16,8 +17,6 @@ type KeptnMetricProvider struct {
 	K8sClient client.Client
 }
 
-const KLTNamespace = "keptn-lifecycle-toolkit-system"
-
 // FetchData fetches the SLI values from KeptnMetric resource
 func (p *KeptnMetricProvider) FetchData(ctx context.Context, objective klcv1alpha3.Objective, namespace string) (string, []byte, error) {
 	metric, err := p.GetKeptnMetric(ctx, objective, namespace)
@@ -45,8 +44,8 @@ func (p *KeptnMetricProvider) GetKeptnMetric(ctx context.Context, objective klcv
 	} else {
 		if err := p.K8sClient.Get(ctx, types.NamespacedName{Name: objective.KeptnMetricRef.Name, Namespace: namespace}, metric); err != nil {
 			p.Log.Error(err, "Failed to get KeptnMetric from KeptnEvaluation resource namespace")
-			if err := p.K8sClient.Get(ctx, types.NamespacedName{Name: objective.KeptnMetricRef.Name, Namespace: KLTNamespace}, metric); err != nil {
-				p.Log.Error(err, "Failed to get KeptnMetric from "+KLTNamespace+" namespace")
+			if err := p.K8sClient.Get(ctx, types.NamespacedName{Name: objective.KeptnMetricRef.Name, Namespace: common.KLTNamespace}, metric); err != nil {
+				p.Log.Error(err, "Failed to get KeptnMetric from "+common.KLTNamespace+" namespace")
 				return nil, err
 			}
 		}

operator/controllers/common/providers/keptnmetric/keptnmetric_test.go

@@ -6,6 +6,7 @@ import (
 
 	metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1alpha2"
 	klcv1alpha3 "github.com/keptn/lifecycle-toolkit/operator/apis/lifecycle/v1alpha3"
+	"github.com/keptn/lifecycle-toolkit/operator/controllers/common"
 	"github.com/stretchr/testify/require"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -170,14 +171,14 @@ func Test_Getkeptnmetric(t *testing.T) {
 			metric: &metricsapi.KeptnMetric{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "metric",
-					Namespace: KLTNamespace,
+					Namespace: common.KLTNamespace,
 				},
 			},
 			namespace: "my-other-namespace",
 			out: &metricsapi.KeptnMetric{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "metric",
-					Namespace: KLTNamespace,
+					Namespace: common.KLTNamespace,
 				},
 			},
 			wantError: false,

operator/controllers/common/taskhandler.go

@@ -159,8 +159,7 @@ func (r TaskHandler) setupTasks(taskCreateAttributes CreateTaskAttributes, piWra
 }
 
 func (r TaskHandler) handleTaskNotExists(ctx context.Context, phaseCtx context.Context, taskCreateAttributes CreateTaskAttributes, taskName string, piWrapper *interfaces.PhaseItemWrapper, reconcileObject client.Object, task *klcv1alpha3.KeptnTask, taskStatus *klcv1alpha3.ItemStatus) error {
-	definition := &klcv1alpha3.KeptnTaskDefinition{}
-	err := r.Client.Get(ctx, types.NamespacedName{Name: taskName, Namespace: piWrapper.GetNamespace()}, definition)
+	definition, err := GetTaskDefinition(r.Client, r.Log, ctx, taskName, piWrapper.GetNamespace())
 	if err != nil {
 		r.Log.Error(err, "could not find KeptnTaskDefinition")
 		return controllererrors.ErrCannotGetKeptnTaskDefinition

operator/controllers/common/taskhandler_test.go

@@ -87,6 +87,46 @@ func TestTaskHandler(t *testing.T) {
 			getSpanCalls:    0,
 			unbindSpanCalls: 0,
 		},
+		{
+			name: "task not started - taskDefinition in default KLT namespace",
+			object: &v1alpha3.KeptnAppVersion{
+				ObjectMeta: v1.ObjectMeta{
+					Namespace: "namespace",
+				},
+				Spec: v1alpha3.KeptnAppVersionSpec{
+					KeptnAppSpec: v1alpha3.KeptnAppSpec{
+						PreDeploymentTasks: []string{"task-def"},
+					},
+				},
+			},
+			taskDef: &v1alpha3.KeptnTaskDefinition{
+				ObjectMeta: v1.ObjectMeta{
+					Namespace: KLTNamespace,
+					Name:      "task-def",
+				},
+			},
+			taskObj: v1alpha3.KeptnTask{},
+			createAttr: CreateTaskAttributes{
+				SpanName: "",
+				Definition: v1alpha3.KeptnTaskDefinition{
+					ObjectMeta: v1.ObjectMeta{
+						Name: "task-def",
+					},
+				},
+				CheckType: apicommon.PreDeploymentCheckType,
+			},
+			wantStatus: []v1alpha3.ItemStatus{
+				{
+					DefinitionName: "task-def",
+					Status:         apicommon.StatePending,
+					Name:           "pre-task-def-",
+				},
+			},
+			wantSummary:     apicommon.StatusSummary{Total: 1, Pending: 1},
+			wantErr:         nil,
+			getSpanCalls:    1,
+			unbindSpanCalls: 0,
+		},
 		{
 			name: "task not started",
 			object: &v1alpha3.KeptnAppVersion{

operator/controllers/lifecycle/keptntask/job_utils.go

@@ -16,7 +16,7 @@ import (
 
 func (r *KeptnTaskReconciler) createJob(ctx context.Context, req ctrl.Request, task *klcv1alpha3.KeptnTask) error {
 	jobName := ""
-	definition, err := r.getTaskDefinition(ctx, task.Spec.TaskDefinition, req.Namespace)
+	definition, err := controllercommon.GetTaskDefinition(r.Client, r.Log, ctx, task.Spec.TaskDefinition, req.Namespace)
 	if err != nil {
 		controllercommon.RecordEvent(r.Recorder, apicommon.PhaseCreateTask, "Warning", task, "TaskDefinitionNotFound", fmt.Sprintf("could not find KeptnTaskDefinition: %s ", task.Spec.TaskDefinition), "")
 		return err
@@ -129,7 +129,7 @@ func setupTaskContext(task *klcv1alpha3.KeptnTask) klcv1alpha3.TaskContext {
 
 func (r *KeptnTaskReconciler) handleParent(ctx context.Context, req ctrl.Request, task *klcv1alpha3.KeptnTask, definition *klcv1alpha3.KeptnTaskDefinition, params FunctionExecutionParams) error {
 	var parentJobParams FunctionExecutionParams
-	parentDefinition, err := r.getTaskDefinition(ctx, definition.Spec.Function.FunctionReference.Name, req.Namespace)
+	parentDefinition, err := controllercommon.GetTaskDefinition(r.Client, r.Log, ctx, definition.Spec.Function.FunctionReference.Name, req.Namespace)
 	if err != nil {
 		controllercommon.RecordEvent(r.Recorder, apicommon.PhaseCreateTask, "Warning", task, "TaskDefinitionNotFound", fmt.Sprintf("could not find KeptnTaskDefinition: %s ", task.Spec.TaskDefinition), "")
 		return err

operator/controllers/lifecycle/keptntask/job_utils_test.go

@@ -6,6 +6,7 @@ import (
 
 	klcv1alpha3 "github.com/keptn/lifecycle-toolkit/operator/apis/lifecycle/v1alpha3"
 	apicommon "github.com/keptn/lifecycle-toolkit/operator/apis/lifecycle/v1alpha3/common"
+	"github.com/keptn/lifecycle-toolkit/operator/controllers/common"
 	"github.com/stretchr/testify/require"
 	batchv1 "k8s.io/api/batch/v1"
 	v1 "k8s.io/api/core/v1"
@@ -90,6 +91,80 @@ func TestKeptnTaskReconciler_createJob(t *testing.T) {
 	}, resultingJob.Annotations)
 }
 
+func TestKeptnTaskReconciler_createJob_withTaskDefInDefaultNamespace(t *testing.T) {
+	namespace := "default"
+	cmName := "my-cmd"
+	taskDefinitionName := "my-task-definition"
+
+	cm := makeConfigMap(cmName, namespace)
+
+	fakeClient := fake.NewClientBuilder().WithObjects(cm).Build()
+
+	fakeRecorder := &record.FakeRecorder{}
+
+	err := klcv1alpha3.AddToScheme(fakeClient.Scheme())
+	require.Nil(t, err)
+
+	taskDefinition := makeTaskDefinitionWithConfigmapRef(taskDefinitionName, common.KLTNamespace, cmName)
+
+	err = fakeClient.Create(context.TODO(), taskDefinition)
+	require.Nil(t, err)
+
+	taskDefinition.Status.Function.ConfigMap = cmName
+	err = fakeClient.Status().Update(context.TODO(), taskDefinition)
+	require.Nil(t, err)
+
+	r := &KeptnTaskReconciler{
+		Client:   fakeClient,
+		Recorder: fakeRecorder,
+		Log:      ctrl.Log.WithName("task-controller"),
+		Scheme:   fakeClient.Scheme(),
+	}
+
+	task := makeTask("my-task", namespace, taskDefinitionName)
+
+	err = fakeClient.Create(context.TODO(), task)
+	require.Nil(t, err)
+
+	req := ctrl.Request{
+		NamespacedName: types.NamespacedName{
+			Namespace: namespace,
+		},
+	}
+
+	// retrieve the task again to verify its status
+	err = fakeClient.Get(context.TODO(), types.NamespacedName{
+		Namespace: namespace,
+		Name:      task.Name,
+	}, task)
+
+	require.Nil(t, err)
+
+	err = r.createJob(context.TODO(), req, task)
+	require.Nil(t, err)
+
+	require.NotEmpty(t, task.Status.JobName)
+
+	resultingJob := &batchv1.Job{}
+	err = fakeClient.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: task.Status.JobName}, resultingJob)
+	require.Nil(t, err)
+
+	require.Equal(t, namespace, resultingJob.Namespace)
+	require.NotEmpty(t, resultingJob.OwnerReferences)
+	require.Len(t, resultingJob.Spec.Template.Spec.Containers, 1)
+	require.Len(t, resultingJob.Spec.Template.Spec.Containers[0].Env, 4)
+	require.Equal(t, map[string]string{
+		"label1":             "label2",
+		"keptn.sh/app":       "my-app",
+		"keptn.sh/task-name": "my-task",
+		"keptn.sh/version":   "",
+		"keptn.sh/workload":  "my-workload",
+	}, resultingJob.Labels)
+	require.Equal(t, map[string]string{
+		"annotation1": "annotation2",
+	}, resultingJob.Annotations)
+}
+
 func TestKeptnTaskReconciler_updateJob(t *testing.T) {
 	namespace := "default"
 	taskDefinitionName := "my-task-definition"