feat(operator): add logic to keptnmetrics controller (#647)

operator/config/crd/bases/metrics.keptn.sh_keptnmetrics.yaml

@@ -16,7 +16,7 @@ spec:
   scope: Namespaced
   versions:
   - additionalPrinterColumns:
-    - jsonPath: .spec.source
+    - jsonPath: .spec.provider.name
       name: Provider
       type: string
     - jsonPath: .spec.query

operator/config/rbac/extra_role_binding.yaml

@@ -0,0 +1,13 @@
+## This is not autogenerated
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: manager-rolebinding
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: manager-role
+subjects:
+  - kind: ServiceAccount
+    name: controller-manager
+    namespace: system

operator/config/rbac/kustomization.yaml

@@ -7,6 +7,7 @@ resources:
 - service_account.yaml
 - role.yaml
 - role_binding.yaml
+- extra_role_binding.yaml
 - leader_election_role.yaml
 - leader_election_role_binding.yaml
 # Comment the following 4 lines if you want to disable

operator/config/rbac/role.yaml

@@ -79,12 +79,6 @@ rules:
   - get
   - list
   - watch
-- apiGroups:
-  - ""
-  resources:
-  - secrets
-  verbs:
-  - get
 - apiGroups:
   - lifecycle.keptn.sh
   resources:
@@ -314,12 +308,8 @@ rules:
   resources:
   - keptnmetrics
   verbs:
-  - create
-  - delete
   - get
   - list
-  - patch
-  - update
   - watch
 - apiGroups:
   - metrics.keptn.sh
@@ -335,3 +325,25 @@ rules:
   - get
   - patch
   - update
+- apiGroups:
+  - metrics.keptn.sh
+  resources:
+  - providers
+  verbs:
+  - get
+  - list
+  - watch
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  creationTimestamp: null
+  name: manager-role
+  namespace: keptn-lifecycle-toolkit-system
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - get

operator/controllers/lifecycle/keptnevaluation/providers/dynatrace.go → operator/controllers/common/providers/dynatrace.go

@@ -38,7 +38,7 @@ type DynatraceData struct {
 	Values     []*float64 `json:"values"`
 }
 
-func (d *KeptnDynatraceProvider) EvaluateQuery(ctx context.Context, objective klcv1alpha2.Objective, provider klcv1alpha2.KeptnEvaluationProvider) (string, error) {
+func (d *KeptnDynatraceProvider) EvaluateQuery(ctx context.Context, objective klcv1alpha2.Objective, provider klcv1alpha2.KeptnEvaluationProvider) (string, []byte, error) {
 	qURL := provider.Spec.TargetServer + "/api/v2/metrics/query?metricSelector=" + objective.Query
 
 	d.Log.Info("Running query: " + qURL)
@@ -47,19 +47,19 @@ func (d *KeptnDynatraceProvider) EvaluateQuery(ctx context.Context, objective kl
 	req, err := http.NewRequestWithContext(ctx, "GET", qURL, nil)
 	if err != nil {
 		d.Log.Error(err, "Error while creating request")
-		return "", err
+		return "", nil, err
 	}
 
 	token, err := d.getDTApiToken(ctx, provider)
 	if err != nil {
-		return "", err
+		return "", nil, err
 	}
 
 	req.Header.Set("Authorization", "Api-Token "+token)
 	res, err := d.httpClient.Do(req)
 	if err != nil {
 		d.Log.Error(err, "Error while creating request")
-		return "", err
+		return "", nil, err
 	}
 	defer func() {
 		err := res.Body.Close()
@@ -74,10 +74,11 @@ func (d *KeptnDynatraceProvider) EvaluateQuery(ctx context.Context, objective kl
 	err = json.Unmarshal(b, &result)
 	if err != nil {
 		d.Log.Error(err, "Error while parsing response")
-		return "", err
+		return "", nil, err
 	}
 
-	return fmt.Sprintf("%f", d.getSingleValue(result)), nil
+	r := fmt.Sprintf("%f", d.getSingleValue(result))
+	return r, b, nil
 }
 
 func (d *KeptnDynatraceProvider) getSingleValue(result DynatraceResponse) float64 {

operator/controllers/lifecycle/keptnevaluation/providers/dynatrace_test.go → operator/controllers/common/providers/dynatrace_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	ctrl "sigs.k8s.io/controller-runtime"
 )
@@ -133,7 +134,11 @@ func TestEvaluateQuery_CorrectHTTP(t *testing.T) {
 			TargetServer: svr.URL,
 		},
 	}
-	_, _ = kdp.EvaluateQuery(context.TODO(), obj, p)
+	r, raw, e := kdp.EvaluateQuery(context.TODO(), obj, p)
+	require.True(t, errors.IsNotFound(e))
+	require.Equal(t, []byte(nil), raw)
+	require.Equal(t, "", r)
+
 }
 
 func TestEvaluateQuery_WrongPayloadHandling(t *testing.T) {
@@ -173,8 +178,9 @@ func TestEvaluateQuery_WrongPayloadHandling(t *testing.T) {
 			TargetServer: svr.URL,
 		},
 	}
-	r, e := kdp.EvaluateQuery(context.TODO(), obj, p)
+	r, raw, e := kdp.EvaluateQuery(context.TODO(), obj, p)
 	require.Equal(t, "", r)
+	require.Equal(t, []byte(nil), raw)
 	require.NotNil(t, e)
 }
 
@@ -199,7 +205,7 @@ func TestEvaluateQuery_MissingSecret(t *testing.T) {
 			TargetServer: svr.URL,
 		},
 	}
-	_, e := kdp.EvaluateQuery(context.TODO(), obj, p)
+	_, _, e := kdp.EvaluateQuery(context.TODO(), obj, p)
 	require.NotNil(t, e)
 	require.True(t, strings.Contains(e.Error(), "the SecretKeyRef property with the DT API token is missing"))
 }
@@ -231,9 +237,9 @@ func TestEvaluateQuery_SecretNotFound(t *testing.T) {
 			TargetServer: svr.URL,
 		},
 	}
-	_, e := kdp.EvaluateQuery(context.TODO(), obj, p)
+	_, _, e := kdp.EvaluateQuery(context.TODO(), obj, p)
 	require.NotNil(t, e)
-	require.True(t, strings.Contains(e.Error(), "not found"))
+	require.True(t, errors.IsNotFound(e))
 }
 
 func TestEvaluateQuery_RefNotExistingKey(t *testing.T) {
@@ -275,7 +281,7 @@ func TestEvaluateQuery_RefNotExistingKey(t *testing.T) {
 		},
 	}
 
-	_, e := kdp.EvaluateQuery(context.TODO(), obj, p)
+	_, _, e := kdp.EvaluateQuery(context.TODO(), obj, p)
 	require.NotNil(t, e)
 	require.True(t, strings.Contains(e.Error(), "invalid key "+missingKey))
 }
@@ -317,7 +323,8 @@ func TestEvaluateQuery_HappyPath(t *testing.T) {
 			TargetServer: svr.URL,
 		},
 	}
-	r, e := kdp.EvaluateQuery(context.TODO(), obj, p)
+	r, raw, e := kdp.EvaluateQuery(context.TODO(), obj, p)
 	require.Nil(t, e)
+	require.Equal(t, []byte(dtpayload), raw)
 	require.Equal(t, fmt.Sprintf("%f", 50.0), r)
 }

operator/controllers/lifecycle/keptnevaluation/providers/prometheus.go → operator/controllers/common/providers/prometheus.go

@@ -18,15 +18,15 @@ type KeptnPrometheusProvider struct {
 	httpClient http.Client
 }
 
-func (r *KeptnPrometheusProvider) EvaluateQuery(ctx context.Context, objective klcv1alpha2.Objective, provider klcv1alpha2.KeptnEvaluationProvider) (string, error) {
+func (r *KeptnPrometheusProvider) EvaluateQuery(ctx context.Context, objective klcv1alpha2.Objective, provider klcv1alpha2.KeptnEvaluationProvider) (string, []byte, error) {
 	ctx, cancel := context.WithTimeout(ctx, 20*time.Second)
 	defer cancel()
 
 	queryTime := time.Now().UTC()
 	r.Log.Info("Running query: /api/v1/query?query=" + objective.Query + "&time=" + queryTime.String())
 	client, err := promapi.NewClient(promapi.Config{Address: provider.Spec.TargetServer, Client: &r.httpClient})
 	if err != nil {
-		return "", err
+		return "", nil, err
 	}
 	api := prometheus.NewAPI(client)
 	result, w, err := api.Query(
@@ -37,7 +37,7 @@ func (r *KeptnPrometheusProvider) EvaluateQuery(ctx context.Context, objective k
 	)
 
 	if err != nil {
-		return "", err
+		return "", nil, err
 	}
 
 	if len(w) != 0 {
@@ -47,18 +47,22 @@ func (r *KeptnPrometheusProvider) EvaluateQuery(ctx context.Context, objective k
 	// check if we can cast the result to a vector, it might be another data struct which we can't process
 	resultVector, ok := result.(model.Vector)
 	if !ok {
-		return "", fmt.Errorf("could not cast result")
+		return "", nil, fmt.Errorf("could not cast result")
 	}
 
 	// We are only allowed to return one value, if not the query may be malformed
 	// we are using two different errors to give the user more information about the result
 	if len(resultVector) == 0 {
 		r.Log.Info("No values in query result")
-		return "", fmt.Errorf("no values in query result")
+		return "", nil, fmt.Errorf("no values in query result")
 	} else if len(resultVector) > 1 {
 		r.Log.Info("Too many values in the query result")
-		return "", fmt.Errorf("too many values in the query result")
+		return "", nil, fmt.Errorf("too many values in the query result")
 	}
 	value := resultVector[0].Value.String()
-	return value, nil
+	b, err := resultVector[0].Value.MarshalJSON()
+	if err != nil {
+		return "", nil, err
+	}
+	return value, b, nil
 }

operator/controllers/lifecycle/keptnevaluation/providers/prometheus_test.go → operator/controllers/common/providers/prometheus_test.go

@@ -23,6 +23,7 @@ func Test_prometheus(t *testing.T) {
 		name      string
 		in        string
 		out       string
+		outraw    []byte
 		wantError bool
 	}{
 		{
@@ -35,6 +36,7 @@ func Test_prometheus(t *testing.T) {
 			name:      "warnings",
 			in:        promWarnPayload,
 			out:       "1",
+			outraw:    []byte("\"1\""),
 			wantError: false,
 		},
 		{
@@ -59,6 +61,7 @@ func Test_prometheus(t *testing.T) {
 			name:      "happy path",
 			in:        promPayload,
 			out:       "1",
+			outraw:    []byte("\"1\""),
 			wantError: false,
 		},
 	}
@@ -89,8 +92,9 @@ func Test_prometheus(t *testing.T) {
 					TargetServer: svr.URL,
 				},
 			}
-			r, e := kpp.EvaluateQuery(context.TODO(), obj, p)
+			r, raw, e := kpp.EvaluateQuery(context.TODO(), obj, p)
 			require.Equal(t, tt.out, r)
+			require.Equal(t, tt.outraw, raw)
 			if tt.wantError != (e != nil) {
 				t.Errorf("want error: %t, got: %v", tt.wantError, e)
 			}

operator/controllers/lifecycle/keptnevaluation/providers/provider.go → operator/controllers/common/providers/provider.go

@@ -13,7 +13,7 @@ import (
 
 // KeptnSLIProvider is the interface that describes the operations that an SLI provider must implement
 type KeptnSLIProvider interface {
-	EvaluateQuery(ctx context.Context, objective klcv1alpha2.Objective, provider klcv1alpha2.KeptnEvaluationProvider) (string, error)
+	EvaluateQuery(ctx context.Context, objective klcv1alpha2.Objective, provider klcv1alpha2.KeptnEvaluationProvider) (string, []byte, error)
 }
 
 // NewProvider is a factory method that chooses the right implementation of KeptnSLIProvider

operator/controllers/lifecycle/keptnevaluation/controller.go

@@ -24,8 +24,8 @@ import (
 	klcv1alpha2 "github.com/keptn/lifecycle-toolkit/operator/apis/lifecycle/v1alpha2"
 	apicommon "github.com/keptn/lifecycle-toolkit/operator/apis/lifecycle/v1alpha2/common"
 	controllercommon "github.com/keptn/lifecycle-toolkit/operator/controllers/common"
+	"github.com/keptn/lifecycle-toolkit/operator/controllers/common/providers"
 	controllererrors "github.com/keptn/lifecycle-toolkit/operator/controllers/errors"
-	providers "github.com/keptn/lifecycle-toolkit/operator/controllers/lifecycle/keptnevaluation/providers"
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/codes"
 	"go.opentelemetry.io/otel/propagation"
@@ -50,12 +50,15 @@ type KeptnEvaluationReconciler struct {
 	Tracer   trace.Tracer
 }
 
+//clusterrole
 //+kubebuilder:rbac:groups=lifecycle.keptn.sh,resources=keptnevaluations,verbs=get;list;watch;create;update;patch;delete
 //+kubebuilder:rbac:groups=lifecycle.keptn.sh,resources=keptnevaluations/status,verbs=get;update;patch
 //+kubebuilder:rbac:groups=lifecycle.keptn.sh,resources=keptnevaluations/finalizers,verbs=update
 //+kubebuilder:rbac:groups=lifecycle.keptn.sh,resources=keptnevaluationproviders,verbs=get;list;watch
 //+kubebuilder:rbac:groups=lifecycle.keptn.sh,resources=keptnevaluationdefinitions,verbs=get;list;watch
-//+kubebuilder:rbac:groups=core,resources=secrets,verbs=get
+
+//role
+//+kubebuilder:rbac:groups=core,namespace=keptn-lifecycle-toolkit-system,resources=secrets,verbs=get
 
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.
@@ -202,7 +205,7 @@ func (r *KeptnEvaluationReconciler) evaluateObjective(ctx context.Context, evalu
 		return newStatus, statusSummary
 	}
 	// resolving the SLI value
-	value, err := provider.EvaluateQuery(ctx, query, *evaluationProvider)
+	value, _, err := provider.EvaluateQuery(ctx, query, *evaluationProvider)
 	statusItem := &klcv1alpha2.EvaluationStatusItem{
 		Value:  value,
 		Status: apicommon.StateFailed,