updated credential finders logic and added tests

Signed-off-by: Ariadna Rouco <ariadna.rouco@gmail.com>

.gitignore

@@ -19,3 +19,4 @@ server/static/*
 !server/static/.gitkeep
 coverage-output-e2e/
 coverage-output-unit/
+junit-unit-test.xml

metricproviders/datadog/datadog.go

@@ -435,22 +435,26 @@ func findCredentials(logCtx log.Entry, kubeclientset kubernetes.Interface, names
 	finders := []CredentialsFinder{}
 	secretName := metric.Provider.Datadog.SecretRef.Name
 	namespaced := metric.Provider.Datadog.SecretRef.Namespaced
+	credentialsNs := defaults.Namespace()
 
-	if secretName != "" {
-		if namespaced {
-			finders = append(finders, NewSecretFinder(kubeclientset, secretName, namespace))
-		} else {
-			finders = append(finders, NewSecretFinder(kubeclientset, secretName, defaults.Namespace()))
+	if namespaced {
+		credentialsNs = namespace
+		if secretName == "" {
+			return "", "", "", errors.New("secret name is required for namespaced credentials")
 		}
 	}
-	finders = append(finders, NewEnvVariablesFinder(), NewSecretFinder(kubeclientset, DatadogTokensSecretName, defaults.Namespace()))
 
+	if secretName != "" {
+		finders = append(finders, NewSecretFinder(kubeclientset, secretName, credentialsNs))
+	} else {
+		finders = append(finders, NewEnvVariablesFinder(), NewSecretFinder(kubeclientset, DatadogTokensSecretName, defaults.Namespace()))
+	}
 	for _, finder := range finders {
 		address, apiKey, appKey := finder.FindCredentials(logCtx)
 		if address != "" && apiKey != "" && appKey != "" {
 			return address, apiKey, appKey, nil
 		}
 	}
 
-	return "", "", "", errors.New("failed to find the credentials for datadog provider")
+	return "", "", "", errors.New("failed to find the credentials for datadog metrics provider")
 }

metricproviders/datadog/datadogV1_test.go

@@ -46,9 +46,19 @@ func TestRunSuite(t *testing.T) {
 			},
 		},
 	}
+	ddProviderNamespacedTrue := v1alpha1.MetricProvider{
+		Datadog: &v1alpha1.DatadogMetric{
+			Query:    "avg:kubernetes.cpu.user.total{*}",
+			Interval: "10m",
+			SecretRef: v1alpha1.SecretRef{
+				Namespaced: true,
+			},
+		},
+	}
 
 	// Test Cases
 	tests := []struct {
+		name                    string
 		serverURL               string
 		webServerStatus         int
 		webServerResponse       string
@@ -57,6 +67,7 @@ func TestRunSuite(t *testing.T) {
 		expectedValue           string
 		expectedPhase           v1alpha1.AnalysisPhase
 		expectedErrorMessage    string
+		expectedErrorProvider   bool
 		useEnvVarForKeys        bool
 	}{
 		// When last value of time series matches condition then succeed.
@@ -250,6 +261,18 @@ func TestRunSuite(t *testing.T) {
 			expectedValue:           "0.0003332881882246533",
 			expectedPhase:           v1alpha1.AnalysisPhaseSuccessful,
 			useEnvVarForKeys:        false,
+			expectedErrorProvider:   false,
+		},
+		// When secretRef is namespaced true but no secret name passed, expect failure
+		{
+			metric: v1alpha1.Metric{
+				Name:             "foo",
+				SuccessCondition: "result < 0.001",
+				FailureCondition: "result >= 0.001",
+				Provider:         ddProviderNamespacedTrue,
+			},
+			useEnvVarForKeys:      false,
+			expectedErrorProvider: true,
 		},
 	}
 
@@ -347,7 +370,11 @@ func TestRunSuite(t *testing.T) {
 		}
 		namespace := "namespace"
 
-		provider, _ := NewDatadogProvider(*logCtx, fakeClient, namespace, test.metric)
+		provider, err := NewDatadogProvider(*logCtx, fakeClient, namespace, test.metric)
+		assert.Equal(t, err != nil, test.expectedErrorProvider)
+		if test.expectedErrorProvider {
+			continue
+		}
 
 		metricsMetadata := provider.GetMetadata(test.metric)
 		assert.Nil(t, metricsMetadata)

metricproviders/datadog/finders.go

@@ -33,11 +33,17 @@ func (sf *secretFinder) FindCredentials(logCtx log.Entry) (string, string, strin
 	address := ""
 	secret, err := sf.kubeclientset.CoreV1().Secrets(sf.namespace).Get(context.TODO(), sf.secretName, metav1.GetOptions{})
 	if err != nil {
-		logCtx.Debugf("secret %s in namespace %s", sf.namespace, sf.secretName)
+		logCtx.Debugf("error searching for secret %s in namespace %s: %s", sf.secretName, sf.namespace, err.Error())
 		return "", "", ""
 	}
+
 	apiKey := string(secret.Data[DatadogApiKey])
 	appKey := string(secret.Data[DatadogAppKey])
+
+	if apiKey == "" || appKey == "" {
+		logCtx.Debugf("credentials missing in secret %s in namespace %s", sf.secretName, sf.namespace)
+		return "", "", ""
+	}
 	if _, hasAddress := secret.Data[DatadogAddress]; hasAddress {
 		address = string(secret.Data[DatadogAddress])
 	}

metricproviders/datadog/finders_test.go

@@ -0,0 +1,111 @@
+package datadog
+
+import (
+	"fmt"
+	"testing"
+
+	log "github.com/sirupsen/logrus"
+	"github.com/tj/assert"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	k8sfake "k8s.io/client-go/kubernetes/fake"
+	kubetesting "k8s.io/client-go/testing"
+)
+
+const (
+	DatadogUrl = "datadog.com"
+	ApiKey     = "apiKey"
+	AppKey     = "appKey"
+)
+
+func TestRunSuit(t *testing.T) {
+
+	testCases := []struct {
+		name            string
+		secret          *corev1.Secret
+		expectedApiKey  string
+		expectedAppKey  string
+		expectedAddress string
+		expectError     bool
+	}{
+		{
+			name: "When secret valid, should be successful",
+			secret: NewSecretBuilder().
+				WithName("DatadogTokensSecretName").
+				WithData("address", []byte(DatadogUrl)).
+				WithData("api-key", []byte(ApiKey)).
+				WithData("app-key", []byte(AppKey)).
+				Build(),
+			expectedApiKey:  ApiKey,
+			expectedAppKey:  AppKey,
+			expectedAddress: DatadogUrl,
+		},
+		{
+			name: "When secret is found but no data, should return empty values",
+			secret: NewSecretBuilder().
+				WithName("DatadogTokensSecretName").
+				Build(),
+		},
+		{
+			name:        "When secret not found, should return empty values",
+			expectError: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		fakeClient := k8sfake.NewSimpleClientset()
+		fakeClient.PrependReactor("get", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
+			if tc.expectError {
+				return true, nil, fmt.Errorf("api error")
+			}
+			return true, tc.secret, nil
+		})
+
+		secretFinder := NewSecretFinder(fakeClient, "", "")
+		t.Run(tc.name, func(t *testing.T) {
+			logCtx := *log.WithField("test", "test")
+
+			address, apiKey, appKey := secretFinder.FindCredentials(logCtx)
+			assert.Equal(t, tc.expectedAddress, address)
+			assert.Equal(t, tc.expectedApiKey, apiKey)
+			assert.Equal(t, tc.expectedAppKey, appKey)
+		})
+	}
+
+}
+
+// SecretBuilder helps in constructing a corev1.Secret object
+type SecretBuilder struct {
+	name string
+	data map[string][]byte
+}
+
+// NewSecretBuilder creates a new SecretBuilder
+func NewSecretBuilder() *SecretBuilder {
+	return &SecretBuilder{
+		data: make(map[string][]byte),
+	}
+}
+
+// WithName sets the name for the Secret
+func (b *SecretBuilder) WithName(name string) *SecretBuilder {
+	b.name = name
+	return b
+}
+
+// WithData sets the data for the Secret
+func (b *SecretBuilder) WithData(key string, value []byte) *SecretBuilder {
+	b.data[key] = value
+	return b
+}
+
+// Build constructs the corev1.Secret object
+func (b *SecretBuilder) Build() *corev1.Secret {
+	return &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: b.name,
+		},
+		Data: b.data,
+	}
+}